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 <michael.colli...@linaro.org>
* config/arm/neon.md (widen_<us>sum<mode>): New patterns where
mode is VQI to improve mixed mode vectorization.
* config/arm/neon.md (vec_sel_widen_ssum_lo<VQI:mode><VW:mode>3): New
define_insn to match low half of signed vaddw.
* config/arm/neon.md (vec_sel_widen_ssum_hi<VQI:mode><VW:mode>3): New
define_insn to match high half of signed vaddw.
* config/arm/neon.md (vec_sel_widen_usum_lo<VQI:mode><VW:mode>3): New
define_insn to match low half of unsigned vaddw.
* config/arm/neon.md (vec_sel_widen_usum_hi<VQI:mode><VW:mode>3): 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
