Hi Kyrill, I made the following changes based on your comments: 1. I rebased the patch so that it applies cleanly on trunk 2. Fixed the dg-add-options as requested to my new test cases 3. Fixed the GNU style issues identified by ./contrib/check_GNU_style.sh The failure you are seeing on slp-reduc-3.c is a known failure. The test case has a xfail with 'xfail { vect_widen_sum_hi_to_si_pattern' which I added in my patch. Richard Biener resolved some of these issues with PR 68333, but 'slp-reduc-3.c' still fails. I will create a new PR. I retested on the Linaro testing infrastructure with the latest trunk and the only failure is 'slp-reduc-3.c'. Okay for GCC 7? 2016-02-12 Michael Collison * config/arm/neon.md (widen_sum): New patterns where mode is VQI to improve mixed mode vectorization. * config/arm/neon.md (vec_sel_widen_ssum_lo3): New define_insn to match low half of signed vaddw. * config/arm/neon.md (vec_sel_widen_ssum_hi3): New define_insn to match high half of signed vaddw. * config/arm/neon.md (vec_sel_widen_usum_lo3): New define_insn to match low half of unsigned vaddw. * config/arm/neon.md (vec_sel_widen_usum_hi3): New define_insn to match high half of unsigned vaddw. * config/arm/arm.c (arm_simd_vect_par_cnst_half): New function. (arm_simd_check_vect_par_cnst_half_p): Likewise. * config/arm/arm-protos.h (arm_simd_vect_par_cnst_half): Prototype for new function. (arm_simd_check_vect_par_cnst_half_p): Likewise. * config/arm/predicates.md (vect_par_constant_high): Support big endian and simplify by calling arm_simd_check_vect_par_cnst_half (vect_par_constant_low): Likewise. * testsuite/gcc.target/arm/neon-vaddws16.c: New test. * testsuite/gcc.target/arm/neon-vaddws32.c: New test. * testsuite/gcc.target/arm/neon-vaddwu16.c: New test. * testsuite/gcc.target/arm/neon-vaddwu32.c: New test. * testsuite/gcc.target/arm/neon-vaddwu8.c: New test. * testsuite/lib/target-supports.exp (check_effective_target_vect_widen_sum_hi_to_si_pattern): Indicate that arm neon support vector widen sum of HImode TO SImode. On 02/09/2016 09:27 AM, Kyrill Tkachov wrote: > Hi Michael, > > On 17/12/15 00:02, Michael Collison wrote: >> Kyrill, >> >> I have attached a patch that address your comments. The only change I >> would ask you to re-consider renaming is the function 'bool >> aarch32_simd_check_vect_par_cnst_half'. This function was copied from >> the aarch64 port and I thought it as important to match the naming >> for maintenance purposes. I did rename the function to 'bool >> arm_simd_check_vect_par_cnst_half_p'. I changed 'aarch32' to 'arm' >> and added '_p' per you suggestions. Is this okay? >> > > Ok, that's fine with me. > >> I implemented all your other change suggestions. >> > > Thanks, sorry it took a long time to get back to this, I was busy with > regression-fixing patches as we're > in bug-fixing mode... > >> 2015-12-16 Michael Collison >> >> * config/arm/neon.md (widen_sum): New patterns where >> mode is VQI to improve mixed mode vectorization. >> * config/arm/neon.md (vec_sel_widen_ssum_lo3): >> New >> define_insn to match low half of signed vaddw. >> * config/arm/neon.md (vec_sel_widen_ssum_hi3): >> New >> define_insn to match high half of signed vaddw. >> * config/arm/neon.md (vec_sel_widen_usum_lo3): >> New >> define_insn to match low half of unsigned vaddw. >> * config/arm/neon.md (vec_sel_widen_usum_hi3): >> New >> define_insn to match high half of unsigned vaddw. >> * config/arm/arm.c (arm_simd_vect_par_cnst_half): New function. >> (arm_simd_check_vect_par_cnst_half_p): Likewise. >> * config/arm/arm-protos.h (arm_simd_vect_par_cnst_half): Prototype >> for new function. >> (arm_simd_check_vect_par_cnst_half_p): Likewise. >> * config/arm/predicates.md (vect_par_constant_high): Support >> big endian and simplify by calling >> arm_simd_check_vect_par_cnst_half >> (vect_par_constant_low): Likewise. >> * testsuite/gcc.target/arm/neon-vaddws16.c: New test. >> * testsuite/gcc.target/arm/neon-vaddws32.c: New test. >> * testsuite/gcc.target/arm/neon-vaddwu16.c: New test. >> * testsuite/gcc.target/arm/neon-vaddwu32.c: New test. >> * testsuite/gcc.target/arm/neon-vaddwu8.c: New test. >> * testsuite/lib/target-supports.exp >> (check_effective_target_vect_widen_sum_hi_to_si_pattern): Indicate >> that arm neon support vector widen sum of HImode TO SImode. >> > > I've tried this out and I have a few comments. > The arm.c hunk doesn't apply to current trunk anymore due to context. > Can you please rebase the patch? > I've fixed it up manually in my tree so I can build it. > With this patch I'm seeing two PASS->FAIL on arm-none-eabi: > FAIL: gcc.dg/vect/slp-reduc-3.c -flto -ffat-lto-objects > scan-tree-dump-times vect "vectorizing stmts using SLP" 1 > FAIL: gcc.dg/vect/slp-reduc-3.c scan-tree-dump-times vect "vectorizing > stmts using SLP" 1 > My compiler is configured with --with-float=hard --with-cpu=cortex-a9 > --with-fpu=neon --with-mode=thumb > Can you please look into these? Maybe it's just the tests that need > adjustment? > > Also, I'm seeing the new tests give an error: > ERROR: gcc.target/arm/neon-vaddws16.c: Unrecognized option type: > arm_neon_ok for " dg-add-options 3 arm_neon_ok " > UNRESOLVED: gcc.target/arm/neon-vaddws16.c: Unrecognized option type: > arm_neon_ok for " dg-add-options 3 arm_neon_ok " > > That've because the dg-add-options argument should be arm_neon rather > than arm_neon_ok. > Also, since the new tests are compile-only the effective target check > should be arm_neon_ok rather than arm_neon_hw. > > I also see ./contrib/check_GNU_style.sh complaining about some minor > style issues like trailing whitespace and > blocks of whitespace that should be replaced with tabs. > > In any case, this patch is GCC 7 material at this point, so I think > with the above issues resolved > (and the FAILs investigated) this should be in good shape. > > Thanks, > Kyrill -- Michael Collison Linaro Toolchain Working Group michael.collison@linaro.org