Hi Kyrill, I couldn't find a way to actually generate this case so I have instead removed the entry from ANY128. New patch and changelog below.
-- The iterator ANY64 are used in various general split patterns and is supposed to contain all 64 bit modes. For some reason the pattern has HI but not HF. This adds HF so that general 64 bit splits are generated for these modes as well. These are required by various split patterns that expect them to be there. Bootstrapped Regtested on arm-none-gnueabihf and no issues. Ok for trunk? Thanks, Tamar gcc/ChangeLog: 2019-02-14 Tamar Christina <tamar.christ...@arm.com> PR target/88850 * config/arm/iterators.md (ANY64): Add V4HF. gcc/testsuite/ChangeLog: 2019-02-14 Tamar Christina <tamar.christ...@arm.com> PR target/88850 * gcc.target/arm/pr88850-2.c: New test. * lib/target-supports.exp (check_effective_target_arm_neon_softfp_fp16_ok_nocache, check_effective_target_arm_neon_softfp_fp16_ok, add_options_for_arm_neon_softfp_fp16): New. The 02/13/2019 10:57, Kyrill Tkachov wrote: > Hi Tamar > > On 2/13/19 10:33 AM, Tamar Christina wrote: > > Hi All, > > > > The iterators ANY64 and ANY128 are used in various general split > > patterns and > > are supposed to contain any 64 bit and 128 bit modes respectively. > > > > For some reason these patterns had HI but not HF. This adds HF so > > that general > > 64 and 128 bit splits are generated for these modes as well. These > > are required > > by various split patterns that expect them to be there. > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and <native regtest > > still running> issues. > > > Please do this on an arm-none-linux-gnueabihf target. > > Though I suspect this is just a placeholder from a boilerplate ;) > > > Ok for trunk? > > > > Thanks, > > Tamar > > > > gcc/ChangeLog: > > > > 2019-02-13 Tamar Christina <tamar.christ...@arm.com> > > > > PR target/88850 > > * config/arm/iterators.md (ANY64): Add V4HF, > > (ANY128): Add V8HF. > > > > gcc/testsuite/ChangeLog: > > > > 2019-02-13 Tamar Christina <tamar.christ...@arm.com> > > > > PR target/88850 > > * gcc.target/arm/pr88850-2.c: New test. > > > > -- > > diff --git a/gcc/config/arm/iterators.md b/gcc/config/arm/iterators.md > index > c33e572c3e89c3dc5848bd6b825d618481247558..4ac048a0c609273691c264c97ccf6cd47b43943b > 100644 > --- a/gcc/config/arm/iterators.md > +++ b/gcc/config/arm/iterators.md > @@ -24,11 +24,11 @@ > > ;;---------------------------------------------------------------------------- > > ;; A list of modes that are exactly 64 bits in size. This is used to expand > -;; some splits that are the same for all modes when operating on ARM > +;; some splits that are the same for all modes when operating on ARM > ;; registers. > -(define_mode_iterator ANY64 [DI DF V8QI V4HI V2SI V2SF]) > +(define_mode_iterator ANY64 [DI DF V8QI V4HI V4HF V2SI V2SF]) > > -(define_mode_iterator ANY128 [V2DI V2DF V16QI V8HI V4SI V4SF]) > +(define_mode_iterator ANY128 [V2DI V2DF V16QI V8HI V8HF V4SI V4SF]) > > > I see ANY128 is used in the move_hi_quad_<mode> and move_lo_quad_<mode> > expanders in neon.md. > Does the use of HF modes need guarding by TARGET_NEON_FP16INST there? > Would be good to see a test exercising this change... > > > ;; A list of integer modes that are up to one word long > (define_mode_iterator QHSI [QI HI SI]) > diff --git a/gcc/testsuite/gcc.target/arm/pr88850-2.c > b/gcc/testsuite/gcc.target/arm/pr88850-2.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..fee2a0fe3a227ea4652bea2aa0e7335d20a7e9b5 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/pr88850-2.c > @@ -0,0 +1,19 @@ > +/* PR target/88850. */ > +/* { dg-do compile } */ > +/* { dg-additional-options "-O2 -march=armv7-a -mfloat-abi=softfp > -fdump-rtl-final" } */ > +/* { dg-add-options arm_neon_fp16 } */ > +/* { dg-require-effective-target arm_soft_ok } */ > +/* { dg-require-effective-target arm_neon_fp16_ok } */ > + > > arm_soft_ok checks if it's okay to add -mfloat-abi=soft, but you're adding > softfp. > Is this right? > > Thanks, > Kyrill > > +#include <arm_neon.h> > + > +extern void c (int, float16x4_t); > + > +void a (float16x4_t b) > +{ > + c (0, b); > +} > + > + > +/* Check that these 64-bit moves are done in SI. */ > +/* { dg-final { scan-rtl-dump "_movsi_vfp" "final" } } * > --
diff --git a/gcc/config/arm/iterators.md b/gcc/config/arm/iterators.md index c33e572c3e89c3dc5848bd6b825d618481247558..eb07c5b90c1b1905d35d7b480bdbe7d7a45ab7ba 100644 --- a/gcc/config/arm/iterators.md +++ b/gcc/config/arm/iterators.md @@ -24,9 +24,9 @@ ;;---------------------------------------------------------------------------- ;; A list of modes that are exactly 64 bits in size. This is used to expand -;; some splits that are the same for all modes when operating on ARM +;; some splits that are the same for all modes when operating on ARM ;; registers. -(define_mode_iterator ANY64 [DI DF V8QI V4HI V2SI V2SF]) +(define_mode_iterator ANY64 [DI DF V8QI V4HI V4HF V2SI V2SF]) (define_mode_iterator ANY128 [V2DI V2DF V16QI V8HI V4SI V4SF]) diff --git a/gcc/testsuite/gcc.target/arm/pr88850-2.c b/gcc/testsuite/gcc.target/arm/pr88850-2.c new file mode 100644 index 0000000000000000000000000000000000000000..7a1aec55dc70625bd6306e8d6bf094e11afe81bc --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr88850-2.c @@ -0,0 +1,18 @@ +/* PR target/88850. */ +/* { dg-do compile } */ +/* { dg-additional-options "-O2 -march=armv7-a -fdump-rtl-final" } */ +/* { dg-add-options arm_neon_softfp_fp16 } */ +/* { dg-require-effective-target arm_neon_softfp_fp16_ok } */ + +#include <arm_neon.h> + +extern void c (int, float16x4_t); + +void a (float16x4_t b) +{ + c (0, b); +} + + +/* Check that these 64-bit moves are done in SI. */ +/* { dg-final { scan-rtl-dump "_movsi_vfp" "final" } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index a0b4b99067f9ae225bde3b6bc719e89e1ea8e0e1..1d237d4cd664924cc580cff67a563230b3fe9571 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -3785,6 +3785,44 @@ proc check_effective_target_arm_neon_fp16_ok { } { check_effective_target_arm_neon_fp16_ok_nocache] } +# Return 1 if this is an ARM target supporting -mfpu=neon-fp16 +# and -mfloat-abi=softfp together. Some multilibs may be +# incompatible with these options. Also set et_arm_neon_softfp_fp16_flags to +# the best options to add. + +proc check_effective_target_arm_neon_softfp_fp16_ok_nocache { } { + global et_arm_neon_softfp_fp16_flags + global et_arm_neon_flags + set et_arm_neon_softfp_fp16_flags "" + if { [check_effective_target_arm32] + && [check_effective_target_arm_neon_ok] } { + foreach flags {"-mfpu=neon-fp16 -mfloat-abi=softfp" + "-mfloat-abi=softfp -mfp16-format=ieee" + "-mfpu=neon-fp16 -mfloat-abi=softfp -mfp16-format=ieee"} { + if { [check_no_compiler_messages_nocache arm_neon_softfp_fp16_ok object { + #include "arm_neon.h" + float16x4_t + foo (float32x4_t arg) + { + return vcvt_f16_f32 (arg); + } + } "$et_arm_neon_flags $flags"] } { + set et_arm_neon_softfp_fp16_flags [concat $et_arm_neon_flags $flags] + return 1 + } + } + } + + return 0 +} + +proc check_effective_target_arm_neon_softfp_fp16_ok { } { + return [check_cached_effective_target arm_neon_softfp_fp16_ok \ + check_effective_target_arm_neon_softfp_fp16_ok_nocache] +} + + + proc check_effective_target_arm_neon_fp16_hw { } { if {! [check_effective_target_arm_neon_fp16_ok] } { return 0 @@ -3808,6 +3846,14 @@ proc add_options_for_arm_neon_fp16 { flags } { return "$flags $et_arm_neon_fp16_flags" } +proc add_options_for_arm_neon_softfp_fp16 { flags } { + if { ! [check_effective_target_arm_neon_softfp_fp16_ok] } { + return "$flags" + } + global et_arm_neon_softfp_fp16_flags + return "$flags $et_arm_neon_softfp_fp16_flags" +} + # Return 1 if this is an ARM target supporting the FP16 alternative # format. Some multilibs may be incompatible with the options needed. Also # set et_arm_neon_fp16_flags to the best options to add.