Re: [PATCH][GCC][Arm] Add HF modes to ANY iterators

[Kyrill Tkachov]

On 2/14/19 4:52 PM, Tamar Christina wrote:

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?

Ok.

Thanks,

Kyrill

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" } } *