| Message ID | 5b2cc7ad-df57-56b1-5add-5164072b2d17@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Arm] : Fix no_cond issue introduced by MVE | expand |
Hi Andre, > -----Original Message----- > From: Andre Vieira (lists) <andre.simoesdiavieira@arm.com> > Sent: 19 March 2020 16:52 > To: Christophe Lyon <christophe.lyon@linaro.org>; Kyrylo Tkachov > <Kyrylo.Tkachov@arm.com> > Cc: gcc-patches@gcc.gnu.org; Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> > Subject: [PATCH, GCC, Arm]: Fix no_cond issue introduced by MVE > > Hi, > > This was a matter of mistaken logic in (define_attr "conds" ..). This was > setting the conds attribute for any neon instruction to no_cond which was > messing up code generation. > > Bootsrapped and regression tested arm-linux-gnueabihf. > > Is this OK for trunk? Ok. Thanks, Kyrill > > 2020-03-19 Andre Vieira <andre.simoesdiasvieira@arm.com> > > * config/arm/arm.md (define_attr "conds"): Fix logic for neon and mve. > > On 17/03/2020 11:55, Christophe Lyon via Gcc-patches wrote: > > On Mon, 16 Mar 2020 at 18:41, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> > wrote: > >> Hi Srinath, > >> > >> I've pushed the first three of the patches for you to master: > >> [ARM][GCC][3/x]: MVE ACLE intrinsics framework patch. > >> [ARM][GCC][2/x]: MVE ACLE intrinsics framework patch. > >> [ARM][GCC][1/x]: MVE ACLE intrinsics framework patch. > >> > >> For this first one I adjusted the add_options directives for mve to actually > add armv8.1-m.main+mve or armv8.1-m.main+mve.fp to the march line so > that the tests don't appear as UNSUPPORTED when running the testsuite > without any explicit options specified. > >> Please keep an eye out for any fallout in the coming days (though > >> I've smoked tested the patches myself before committing) > >> > > Hi, > > > > I've noticed regressions on arm-none-linux-gnueabihf --with-mode arm > > --with-cpu cortex-a9 --with-fpu neon-fp16 (--with-mode arm is > > important, or force -marm when running the tests) > > FAIL: gcc.target/arm/neon-cond-1.c execution test > > FAIL: gfortran.dg/array_constructor_7.f90 -O3 -g execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O0 execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O1 execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O2 execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O3 -fomit-frame-pointer > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution > > test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -O3 -g execution test > > FAIL: gfortran.dg/complex_intrinsic_5.f90 -Os execution test > > > > Christophe > > > >> Thanks, > >> Kyrill > >> > >> -----Original Message----- > >> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of > >> Srinath Parvathaneni > >> Sent: 16 March 2020 10:53 > >> To: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>; > >> gcc-patches@gcc.gnu.org > >> Subject: Re: [PATCH v3][ARM][GCC][1/x]: MVE ACLE intrinsics framework > patch. > >> > >> Hi Kyrill, > >> > >>> This is ok but please bootstrap it on arm-none-linux-gnueabihf as well. > >> I have bootstrapped this patch on arm-none-linux-gnueabihf and found > no issues. > >> There is problem with git commit rights, could you commit this patch on > my behalf. > >> > >> Regards > >> SRI > >> ________________________________ > >> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> > >> Sent: 12 March 2020 11:15 > >> To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>; > >> gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org> > >> Subject: Re: [PATCH v3][ARM][GCC][1/x]: MVE ACLE intrinsics framework > patch. > >> > >> Hi Srinath, > >> > >> On 3/10/20 6:19 PM, Srinath Parvathaneni wrote: > >>> Hello Kyrill, > >>> > >>> This patch addresses all the comments in patch version v2. > >>> (version v2) > >>> https://gcc.gnu.org/pipermail/gcc-patches/2020-February/540415.html > >>> > >>> #### > >>> > >>> Hello, > >>> > >>> This patch creates the required framework for MVE ACLE intrinsics. > >>> > >>> The following changes are done in this patch to support MVE ACLE > >>> intrinsics. > >>> > >>> Header file arm_mve.h is added to source code, which contains the > >>> definitions of MVE ACLE intrinsics and different data types used in > >>> MVE. Machine description file mve.md is also added which contains > >>> the RTL patterns defined for MVE. > >>> > >>> A new reigster "p0" is added which is used in by MVE predicated > >>> patterns. A new register class "VPR_REG" > >>> is added and its contents are defined in REG_CLASS_CONTENTS. > >>> > >>> The vec-common.md file is modified to support the standard move > >>> patterns. The prefix of neon functions which are also used by MVE is > >>> changed from "neon_" to "simd_". > >>> eg: neon_immediate_valid_for_move changed to > >>> simd_immediate_valid_for_move. > >>> > >>> In the patch standard patterns mve_move, mve_store and move_load > for > >>> MVE are added and neon.md and vfp.md files are modified to support > >>> this common patterns. > >>> > >>> Please refer to Arm reference manual [1] for more details. > >>> > >>> [1] https://developer.arm.com/docs/ddi0553/latest > >>> > >>> Regression tested on target arm-none-eabi and armeb-none-eabi and > >>> found no regressions. > >>> > >>> Ok for trunk? > >> > >> This is ok but please bootstrap it on arm-none-linux-gnueabihf as well. > >> > >> Thanks, > >> > >> Kyrill > >> > >> > >>> Thanks, > >>> Srinath > >>> > >>> gcc/ChangeLog: > >>> > >>> 2020-03-06 Andre Vieira <andre.simoesdiasvieira@arm.com> > >>> Mihail Ionescu <mihail.ionescu@arm.com> > >>> Srinath Parvathaneni <srinath.parvathaneni@arm.com> > >>> > >>> * config.gcc (arm_mve.h): Include mve intrinsics header file. > >>> * config/arm/aout.h (p0): Add new register name for MVE > predicated > >>> cases. > >>> * config/arm-builtins.c (ARM_BUILTIN_SIMD_LANE_CHECK): > >>> Define macro > >>> common to Neon and MVE. > >>> (ARM_BUILTIN_NEON_LANE_CHECK): Renamed to > >>> ARM_BUILTIN_SIMD_LANE_CHECK. > >>> (arm_init_simd_builtin_types): Disable poly types for MVE. > >>> (arm_init_neon_builtins): Move a check to arm_init_builtins > >>> function. > >>> (arm_init_builtins): Use ARM_BUILTIN_SIMD_LANE_CHECK instead > of > >>> ARM_BUILTIN_NEON_LANE_CHECK. > >>> (mve_dereference_pointer): Add function. > >>> (arm_expand_builtin_args): Call to mve_dereference_pointer > >>> when MVE is > >>> enabled. > >>> (arm_expand_neon_builtin): Moved to arm_expand_builtin > function. > >>> (arm_expand_builtin): Moved from arm_expand_neon_builtin > function. > >>> * config/arm/arm-c.c (__ARM_FEATURE_MVE): Define macro for > >>> MVE and MVE > >>> with floating point enabled. > >>> * config/arm/arm-protos.h (neon_immediate_valid_for_move): > >>> Renamed to > >>> simd_immediate_valid_for_move. > >>> (simd_immediate_valid_for_move): Renamed from > >>> neon_immediate_valid_for_move function. > >>> * config/arm/arm.c (arm_options_perform_arch_sanity_checks): > >>> Generate > >>> error if vfpv2 feature bit is disabled and mve feature bit is also > >>> disabled for HARD_FLOAT_ABI. > >>> (use_return_insn): Check to not push VFP regs for MVE. > >>> (aapcs_vfp_allocate): Add MVE check to have same Procedure > >>> Call Standard > >>> as Neon. > >>> (aapcs_vfp_allocate_return_reg): Likewise. > >>> (thumb2_legitimate_address_p): Check to return 0 on valid Thumb- > 2 > >>> address operand for MVE. > >>> (arm_rtx_costs_internal): MVE check to determine cost of rtx. > >>> (neon_valid_immediate): Rename to simd_valid_immediate. > >>> (simd_valid_immediate): Rename from neon_valid_immediate. > >>> (simd_valid_immediate): MVE check on size of vector is 128 bits. > >>> (neon_immediate_valid_for_move): Rename to > >>> simd_immediate_valid_for_move. > >>> (simd_immediate_valid_for_move): Rename from > >>> neon_immediate_valid_for_move. > >>> (neon_immediate_valid_for_logic): Modify call to > >>> neon_valid_immediate > >>> function. > >>> (neon_make_constant): Modify call to neon_valid_immediate > >>> function. > >>> (neon_vector_mem_operand): Return VFP register for POST_INC > >>> or PRE_DEC > >>> for MVE. > >>> (output_move_neon): Add MVE check to generate vldm/vstm > >>> instrcutions. > >>> (arm_compute_frame_layout): Calculate space for saved VFP > >>> registers for > >>> MVE. > >>> (arm_save_coproc_regs): Save coproc registers for MVE. > >>> (arm_print_operand): Add case 'E' to print memory operands > >>> for MVE. > >>> (arm_print_operand_address): Check to print register number > >>> for MVE. > >>> (arm_hard_regno_mode_ok): Check for arm hard regno mode ok > >>> for MVE. > >>> (arm_modes_tieable_p): Check to allow structure mode for MVE. > >>> (arm_regno_class): Add VPR_REGNUM check. > >>> (arm_expand_epilogue_apcs_frame): MVE check to calculate > >>> epilogue code > >>> for APCS frame. > >>> (arm_expand_epilogue): MVE check for enabling pop instructions in > >>> epilogue. > >>> (arm_print_asm_arch_directives): Modify function to disable > >>> print of > >>> .arch_extension "mve" and "fp" for cases where MVE is enabled > with > >>> "SOFT FLOAT ABI". > >>> (arm_vector_mode_supported_p): Check for modes available in > >>> MVE interger > >>> and MVE floating point. > >>> (arm_array_mode_supported_p): Add TARGET_HAVE_MVE check > for > >>> array mode > >>> pointer support. > >>> (arm_conditional_register_usage): Enable usage of > >>> conditional regsiter > >>> for MVE. > >>> (fixed_regs[VPR_REGNUM]): Enable VPR_REG for MVE. > >>> (arm_declare_function_name): Modify function to disable print of > >>> .arch_extension "mve" and "fp" for cases where MVE is enabled > with > >>> "SOFT FLOAT ABI". > >>> * config/arm/arm.h (TARGET_HAVE_MVE): Disable for soft > >>> float abi and > >>> when target general registers are required. > >>> (TARGET_HAVE_MVE_FLOAT): Likewise. > >>> (FIXED_REGISTERS): Add bit for VFP_REG class which is > >>> enabled in arm.c > >>> for MVE. > >>> (CALL_USED_REGISTERS): Set bit for VFP_REG class in > >>> CALL_USED_REGISTERS > >>> which indicate this is not available for across function calls. > >>> (FIRST_PSEUDO_REGISTER): Modify. > >>> (VALID_MVE_MODE): Define valid MVE mode. > >>> (VALID_MVE_SI_MODE): Define valid MVE SI mode. > >>> (VALID_MVE_SF_MODE): Define valid MVE SF mode. > >>> (VALID_MVE_STRUCT_MODE): Define valid MVE struct mode. > >>> (VPR_REGNUM): Add Vector Predication Register in > >>> arm_regs_in_sequence > >>> for MVE. > >>> (IS_VPR_REGNUM): Macro to check for VPR_REG register. > >>> (REG_ALLOC_ORDER): Add VPR_REGNUM entry. > >>> (enum reg_class): Add VPR_REG entry. > >>> (REG_CLASS_NAMES): Add VPR_REG entry. > >>> * config/arm/arm.md (VPR_REGNUM): Define. > >>> (conds): Check is_mve_type attrbiute to differentiate > >>> "conditional" and > >>> "unconditional" instructions. > >>> (arm_movsf_soft_insn): Modify RTL to not allow for MVE. > >>> (movdf_soft_insn): Modify RTL to not allow for MVE. > >>> (vfp_pop_multiple_with_writeback): Enable for MVE. > >>> (include "mve.md"): Include mve.md file. > >>> * config/arm/arm_mve.h: Add MVE intrinsics head file. > >>> * config/arm/constraints.md (Up): Constraint to enable "p0" > >>> register in MVE > >>> for vector predicated operands. > >>> * config/arm/iterators.md (VNIM1): Define. > >>> (VNINOTM1): Define. > >>> (VHFBF_split): Define > >>> * config/arm/mve.md: New file. > >>> (mve_mov<mode>): Define RTL for move, store and load in MVE. > >>> (mve_mov<mode>): Define move RTL pattern with vec_duplicate > >>> operator for > >>> second operand. > >>> * config/arm/neon.md (neon_immediate_valid_for_move): > Rename with > >>> simd_immediate_valid_for_move. > >>> (neon_mov<mode>): Split pattern and move expand pattern > >>> "movv8hf" which > >>> is common to MVE and NEON to vec-common.md file. > >>> (vec_init<mode><V_elem_l>): Add TARGET_HAVE_MVE check. > >>> * config/arm/predicates.md (vpr_register_operand): Define. > >>> * config/arm/t-arm: Add mve.md file. > >>> * config/arm/types.md (mve_move): Add MVE instructions > mve_move to > >>> attribute "type". > >>> (mve_store): Add MVE instructions mve_store to attribute "type". > >>> (mve_load): Add MVE instructions mve_load to attribute "type". > >>> (is_mve_type): Define attribute. > >>> * config/arm/vec-common.md (mov<mode>): Modify RTL expand > >>> to support > >>> standard move patterns in MVE along with NEON and IWMMXT > with mode > >>> iterator VNIM1. > >>> (mov<mode>): Modify RTL expand to support standard move > >>> patterns in NEON > >>> and IWMMXT with mode iterator V8HF. > >>> (movv8hf): Define RTL expand to support standard "movv8hf" > >>> pattern in > >>> NEON and MVE. > >>> * config/arm/vfp.md (neon_immediate_valid_for_move): Rename > to > >>> simd_immediate_valid_for_move. > >>> > >>> gcc/testsuite/ChangeLog: > >>> > >>> 2020-03-06 Andre Vieira <andre.simoesdiasvieira@arm.com> > >>> Mihail Ionescu <mihail.ionescu@arm.com> > >>> Srinath Parvathaneni <srinath.parvathaneni@arm.com> > >>> > >>> * gcc.target/arm/mve/intrinsics/mve_vector_float.c: New test. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_float1.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_float2.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_int.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_int1.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_int2.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_uint.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_uint1.c: Likewise. > >>> * gcc.target/arm/mve/intrinsics/mve_vector_uint2.c: Likewise. > >>> * gcc.target/arm/mve/mve.exp: New file. > >>> * lib/target-supports.exp: > >>> (check_effective_target_arm_v8_1m_mve_fp_ok_nocache): Proc to > check > >>> armv8.1-m.main+mve.fp and returning corresponding options. > >>> (check_effective_target_arm_v8_1m_mve_fp_ok): Proc to call > >>> check_effective_target_arm_v8_1m_mve_fp_ok_nocache to check > >>> suppoer of > >>> MVE with floating point on the current target. > >>> (add_options_for_arm_v8_1m_mve_fp): Proc to call > >>> check_effective_target_arm_v8_1m_mve_fp_ok to return > corresponding > >>> compiler options for MVE with floating point. > >>> (check_effective_target_arm_v8_1m_mve_ok_nocache): Modify > >>> to test and > >>> return hard float-abi on success.
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 5387f972f5a864a153873f21b9423d28446daefc..b45109ed5292907dd2fb8ea075f994e510676c8f 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -306,10 +306,10 @@ (define_attr "conds" "use,set,clob,unconditional,nocond" (eq_attr "type" "call")) (const_string "clob") (if_then_else - (ior (eq_attr "is_neon_type" "no") - (eq_attr "is_mve_type" "no")) - (const_string "nocond") - (const_string "unconditional")))) + (ior (eq_attr "is_neon_type" "yes") + (eq_attr "is_mve_type" "yes")) + (const_string "unconditional") + (const_string "nocond")))) ; Predicable means that the insn can be conditionally executed based on ; an automatically added predicate (additional patterns are generated by