Re: [PATCH] Fix memory constraint on MVE v[ld/st][2/4] instructions [PR107714]

[Stam Markianos-Wright <stam.markianos-wright@arm.com>]

On 12/12/2022 13:42, Kyrylo Tkachov wrote:
> Hi Stam,
>
>> -----Original Message-----
>> From: Stam Markianos-Wright <Stam.Markianos-Wright@arm.com>
>> Sent: Friday, December 9, 2022 1:32 PM
>> To: gcc-patches@gcc.gnu.org
>> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
>> <Richard.Earnshaw@arm.com>; Ramana Radhakrishnan
>> <ramana.gcc@googlemail.com>; nickc@redhat.com
>> Subject: [PATCH] Fix memory constraint on MVE v[ld/st][2/4] instructions
>> [PR107714]
>>
>> Hi all,
>>
>> In the M-Class Arm-ARM:
>>
>> https://developer.arm.com/documentation/ddi0553/bu/?lang=en
>>
>> these MVE instructions only have '!' writeback variant and at:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107714
>>
>> we found that the Um constraint would also allow through a
>> register offset writeback, resulting in an assembler error.
>>
>> Here I have added a new constraint and predicate for these
>> instructions, which (uniquely, AFAICT), only support a `!` writeback
>> increment by the data size (inside the compiler this is a POST_INC).
>>
>> No regressions in arm-none-eabi with MVE and MVE.FP.
>>
>> Ok for trunk, and backport to GCC11 and GCC12 (testing pending)?
>>
>> Thanks,
>> Stam
>>
>> gcc/ChangeLog:
>>           PR target/107714
>>           * config/arm/arm-protos.h (mve_struct_mem_operand): New
>> protoype.
>>           * config/arm/arm.cc (mve_struct_mem_operand): New function.
>>           * config/arm/constraints.md (Ug): New constraint.
>>           * config/arm/mve.md (mve_vst4q<mode>): Change constraint.
>>           (mve_vst2q<mode>): Likewise.
>>           (mve_vld4q<mode>): Likewise.
>>           (mve_vld2q<mode>): Likewise.
>>           * config/arm/predicates.md (mve_struct_operand): New predicate.
>>
>> gcc/testsuite/ChangeLog:
>>           PR target/107714
>>           * gcc.target/arm/mve/intrinsics/vldst24q_reg_offset.c: New test.
>
> diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
> index e5a36d29c7135943b9bb5ea396f70e2e4beb1e4a..8908b7f5b15ce150685868e78e75280bf32053f1 100644
> --- a/gcc/config/arm/constraints.md
> +++ b/gcc/config/arm/constraints.md
> @@ -474,6 +474,12 @@
>    (and (match_code "mem")
>         (match_test "TARGET_32BIT && arm_coproc_mem_operand (op, FALSE)")))
>   
> +(define_memory_constraint "Ug"
> + "@internal
> +  In Thumb-2 state a valid MVE struct load/store address."
> + (and (match_code "mem")
> +      (match_test "TARGET_HAVE_MVE && mve_struct_mem_operand (op)")))
> +
>
> I think you can define the constraints in terms of the new mve_struct_operand predicate directly (see how we define the "Ua" constraint, for example).
> Ok if that works (and testing passes of course).

Done as discussed and re-tested on all branches. Pushed as:

4269a6567eb991e6838f40bda5be9e3a7972530c to trunk

25edc76f2afba0b4eaf22174d42de042a6969dbe to gcc-12

08842ad274f5e2630994f7c6e70b2d31768107ea to gcc-11

Thank you!
Stam


> Thanks,
> Kyrill
>