arm: Fix fp16 move patterns for base MVE
authorRichard Sandiford <richard.sandiford@arm.com>
Fri, 25 Sep 2020 11:45:25 +0000 (12:45 +0100)
committerRichard Sandiford <richard.sandiford@arm.com>
Fri, 25 Sep 2020 11:45:25 +0000 (12:45 +0100)
commit6abd428605e3a279e533fde1cecbc9735ce03b66
treed2f68871120d9c8a8d2ea05a6c60551450e352c0
parent4dcc7f03b54087638e084ac69d40d7507fe83bd8
arm: Fix fp16 move patterns for base MVE

This patch fixes ICEs in gcc.dg/torture/float16-basic.c for
-march=armv8.1-m.main+mve -mfloat-abi=hard.  The problem was
that an fp16 argument was (rightly) being passed in FPRs,
but the fp16 move patterns only handled GPRs.  LRA then cycled
trying to look for a way of handling the FPR.

It looks like there are three related problems here:

(1) We're using the wrong fp16 move pattern for base MVE.
    *mov<mode>_vfp_<mode>16 (the pattern we use for +mve.fp)
    works for base MVE too.

(2) The fp16 MVE load and store patterns are separate from the
    main move patterns.  The loads and stores should instead be
    alternatives of the main move patterns, so that LRA knows
    what to do with pseudo registers that become stack slots.

(3) The range restrictions for the loads and stores were wrong
    for fp16: we were enforcing a multiple of 4 in [-255*4, 255*4]
    instead of a multiple of 2 in [-255*2, 255*2].

(2) came from a patch to prevent writeback being used for MVE.
That patch also added a Uj constraint to enforce the correct
memory types for MVE.  I think the simplest fix is therefore to merge
the loads and stores back into the main pattern and extend the Uj
constraint so that it acts like Um for non-MVE.

The testcase for that patch was mve-vldstr16-no-writeback.c, whose
main function is:

void
fn1 (__fp16 *pSrc)
{
  __fp16 high;
  __fp16 *pDst = 0;
  unsigned i;
  for (i = 0;; i++)
    if (pSrc[i])
      pDst[i] = high;
}

Fixing (2) causes the store part to fail, not because we're using
writeback, but because we decide to use GPRs to store high (which is
uninitialised, and so gets replaced with zero).  This patch therefore
adds some scan-assembler-nots instead.  (I wondered about changing the
testcase to initialise high, but that seemed like a bad idea for
a regression test.)

For (3): MVE seems to be the only thing to use arm_coproc_mem_operand_wb
(and its various interfaces) for 16-bit scalars: the Neon patterns only
use it for 32-bit scalars.

I've added new tests to try the various FPR alternatives of the
move patterns.  The range of offsets that GCC uses for FPR loads
and stores is the intersection of the range allowed for GPRs and
FPRs, so the tests include GPR<->memory tests as well.

The fp32 and fp64 tests already pass, they're just there for
completeness.

gcc/
* config/arm/arm-protos.h (arm_mve_mode_and_operands_type_check):
Delete.
* config/arm/arm.c (arm_coproc_mem_operand_wb): Use a scale factor
of 2 rather than 4 for 16-bit modes.
(arm_mve_mode_and_operands_type_check): Delete.
* config/arm/constraints.md (Uj): Allow writeback for Neon,
but continue to disallow it for MVE.
* config/arm/arm.md (*arm32_mov<HFBF:mode>): Add !TARGET_HAVE_MVE.
* config/arm/vfp.md (*mov_load_vfp_hf16, *mov_store_vfp_hf16): Fold
back into...
(*mov<mode>_vfp_<mode>16): ...here but use Uj for the FPR memory
constraints.  Use for base MVE too.

gcc/testsuite/
* gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c: Allow
the store to use GPRs instead of FPRs.  Add scan-assembler-nots
for writeback.
* gcc.target/arm/armv8_1m-fp16-move-1.c: New test.
* gcc.target/arm/armv8_1m-fp32-move-1.c: Likewise.
* gcc.target/arm/armv8_1m-fp64-move-1.c: Likewise.
gcc/config/arm/arm-protos.h
gcc/config/arm/arm.c
gcc/config/arm/arm.md
gcc/config/arm/constraints.md
gcc/config/arm/vfp.md
gcc/testsuite/gcc.target/arm/armv8_1m-fp16-move-1.c [new file with mode: 0644]
gcc/testsuite/gcc.target/arm/armv8_1m-fp32-move-1.c [new file with mode: 0644]
gcc/testsuite/gcc.target/arm/armv8_1m-fp64-move-1.c [new file with mode: 0644]
gcc/testsuite/gcc.target/arm/mve/intrinsics/mve-vldstr16-no-writeback.c