Hi Alex,
> -----Original Message-----
> From: Alex Coplan <alex.cop...@arm.com>
> Sent: 19 March 2021 16:45
> To: gcc-patches@gcc.gnu.org
> Cc: ni...@redhat.com; Richard Earnshaw <richard.earns...@arm.com>;
> Ramana Radhakrishnan <ramana.radhakrish...@arm.com>; Kyrylo
> Tkachov <kyrylo.tkac...@arm.com>
> Subject: [PATCH] arm: Fix MVE ICEs with vector moves and -mpure-code
> [PR97252]
>
> Hi all,
>
> This patch fixes around 500 ICEs in the testsuite which can be seen when
> testing with -march=armv8.1-m.main+mve -mfloat-abi=hard -mpure-code
> (leaving the testsuite free of ICEs in this configuration). All of the
> ICEs are in arm_print_operand (which is expecting a mem and gets another
> rtx, e.g. a const_vector) when running the output code for
> *mve_mov<mode> in alternative 4.
>
> The issue is that MVE vector moves were relying on the arm_reorg pass to
> move constant vectors that we can't easily synthesize to the literal
> pool. This doesn't work for -mpure-code where the literal pool is
> disabled. LLVM puts these in .rodata: I've chosen to do the same here.
>
> With this change, for -mpure-code, we no longer want to allow a constant
> on the RHS of a vector load in RA. To achieve this, I added a new
> constraint which matches constants only if the literal pool is
> available.
>
> Testing:
>
> * Bootstrapped and regtested on arm-linux-gnueabihf, no regressions.
>
> * Regression tested a cross compiler configured with
> --with-arch=armv8.1-m.main+mve --with-float=hard with -mpure-code
> in RUNTESTFLAGS. The results here are:
>
> 539 FAIL->PASS
> 1 PASS->FAIL
> 594 UNRESOLVED->FAIL
> 1705 UNRESOLVED->PASS
>
> On manual inspection the PASS->FAIL is really an UNRESOLVED->FAIL and
> the UNRESOLVED->FAILs appear to be testisms (scan-assemblers that
> weren't written with -mpure-code in mind).
>
> * Similarly, using --with-arch=armv8.1-m.main+mve.fp:
>
> 602 FAIL->PASS
> 1 PASS->FAIL
> 594 UNRESOLVED->FAIL
> 1715 UNRESOLVED->PASS
>
> * Regression tested a cross compiler configured with
> --with-arch=armv8.1-m.main+mve --with-float=hard (without
> -mpure-code). The results here are:
>
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -O0 (test for excess
> errors)
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -O1 (test for excess
> errors)
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -O2 -flto -fno-use-
> linker-plugin -flto-partition=none -ffat-lto-objects (test for excess errors)
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -O2 -flto -fuse-linker-
> plugin -fno-fat-lto-objects -ffat-lto-objects (test for excess errors)
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -O2 (test for excess
> errors)
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -O3 -g (test for
> excess errors)
> FAIL->PASS: gcc.target/arm/pure-code/pr94538-2.c -Os (test for excess
> errors)
>
> with no regressions.
>
> OK for trunk and eventual backport to GCC 10?
>
Thanks for doing this.
static rtx
-neon_vdup_constant (rtx vals)
+neon_vdup_constant (rtx vals, bool check_only_p)
Please document the new parameters in the function comment.
Also, other similar functions in the backend use a "generate" parameter to
guard whether to generate code or not.
See arm_gen_constant, for example. I'd rather we were consistent with that
style, so let's call this parameter "generate" and invert the logic.
Ok with those changes.
Thanks,
Kyrill
{
machine_mode mode = GET_MODE (vals);
machine_mode inner_mode = GET_MODE_INNER (mode);
@@ -13046,6 +13049,9 @@ neon_vdup_constant (rtx vals)
vdup.i16). */
return NULL_RTX;
+ if (check_only_p)
+ return x;
+
> Thanks,
> Alex
>
> gcc/ChangeLog:
>
> PR target/97252
> * config/arm/arm-protos.h (neon_make_constant): Add new
> check_only_p
> argument, default to false.
> * config/arm/arm.c (arm_legitimate_constant_p_1): Reject
> COST_VECTORs
> which neon_make_constant can't handle.
> (neon_vdup_constant): Add check_only_p, avoid emitting insns if it's
> set.
> (neon_make_constant): Plumb new check_only_p argument through.
> * config/arm/constraints.md (Ui): New. Use it...
> * config/arm/mve.md (*mve_mov<mode>): ... here.
> * config/arm/vec-common.md (movv8hf): Use neon_make_constant
> to
> synthesize constants.
