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 ;)
Oops, yes sorry, used the wrong template.. and forgot to fill in the results..
oh my.
Bootstrapped and regtested on arm-none-linux-gnueabihf and no issues :)
>
> > 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?
I don't think it does, the patterns would generate a V_HALF move which should
match either
mov<mode> with the VH iterator or *neon_mov<mode> with VQXMOV, both of which
already
contain V4HF. This is because I think it's just going to move them around in s
registers without caring what's
in them as the bits are not being manipulated so you don't need fp16
instructions.
> Would be good to see a test exercising this change...
Would you be able to guide me in the direction of how? It seems practically
impossible to generate this pattern from C.
The move_hi_quad and move_lo_quad are inserts, but using neon intrinsics like
vcombine_f16 doesn't generate this RTL pattern.
The only thing that uses these things is vec_pack_trunc_ and the only thing
that seems to use this is the auto vectorizer.
The autovectorizer won't ever generate a V8HF case because my previous patch to
enable autovectorization of vector HFmodes was
deferred to GCC 10 as the patch series was cannibalized.
I'm more than happy not to change ANY128 if you prefer.
Thanks,
Tamar
>
>
> ;; 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" } } *
>
--
