On 26/11/2025 14:55, Richard Earnshaw (foss) wrote:
> On 26/11/2025 13:16, Christophe Lyon wrote:
>> Operand 1 should be in the same register as operand 0, this patch
>> fixes a typo in the second alternative of these patterns.
>>
>> This fixes a lot of regressions when running the testsuite for an MVE
>> target such as -march=armv8.1-m.main+mve.fp+fp.dp -mfloat-abi=hard.
>>
>> OK for trunk and gcc-15?
>>
>> Thanks,
>>
>> Christophe
>>
>> gcc/ChangeLog:
>>
>> PR target/122858
>> * config/arm/mve.md (mve_asrl_imm, mve_lsll_imm): Fix constraints
>> of operand 1.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR target/122858
>> * gcc.target/arm/mve/pr122858.c: New test.
>
> Are you sure? Alternative 1 (the second one) is expected to go through the
> split pattern which
> should be able to handle operand 0 != operand1.
>
> I suspect the real problem is that the assembler template is incorrect and
> should really be eg
>
> "@
> asrl%?\\t%Q0, %R1, %2
> #"
>
> Since we want to force a late split if necessary.
>
Hmm, no, that's not sufficient. The problem is that the compiler is choosing
alternative 1 over
alternative 0 when the input needs reloading, but alternative 1 isn't suitable
for that case and the split doesn't match. This is because we've used I, but
that's too broad (the splitter only works for shifts of 32 or more (we assume
both registers are trivial to calculate, which is only true for shifts of more
than the register size).
So I think we need a tighter constraint on alternative 1, let's call it Pg' for
now, which accepts constants in the range 32..255.
R.
> R.
>
>> ---
>>
>> gcc/config/arm/mve.md | 4 +--
>> gcc/testsuite/gcc.target/arm/mve/pr122858.c | 40 +++++++++++++++++++++
>> 2 files changed, 42 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/arm/mve/pr122858.c
>>
>> diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md
>> index b29c5f1bcd5..057e115f7a4 100644
>> --- a/gcc/config/arm/mve.md
>> +++ b/gcc/config/arm/mve.md
>> @@ -4761,7 +4761,7 @@ (define_expand "mve_asrl"
>> ;; we have to split the insn if the amount is not in the [1..32] range
>> (define_insn_and_split "mve_asrl_imm"
>> [(set (match_operand:DI 0 "arm_general_register_operand" "=r,r")
>> - (ashiftrt:DI (match_operand:DI 1 "arm_general_register_operand" "0,r")
>> + (ashiftrt:DI (match_operand:DI 1 "arm_general_register_operand" "0,0")
>> (match_operand:QI 2 "immediate_operand" "Pg,I")))]
>> "TARGET_HAVE_MVE"
>> "asrl%?\\t%Q0, %R1, %2"
>> @@ -4851,7 +4851,7 @@ (define_expand "mve_lsll"
>> ;; we have to split the insn if the amount is not in the [1..32] range
>> (define_insn_and_split "mve_lsll_imm"
>> [(set (match_operand:DI 0 "arm_general_register_operand" "=r,r")
>> - (ashift:DI (match_operand:DI 1 "arm_general_register_operand" "0,r")
>> + (ashift:DI (match_operand:DI 1 "arm_general_register_operand" "0,0")
>> (match_operand:QI 2 "immediate_operand" "Pg,I")))]
>> "TARGET_HAVE_MVE"
>> "lsll%?\\t%Q0, %R1, %2"
>> diff --git a/gcc/testsuite/gcc.target/arm/mve/pr122858.c
>> b/gcc/testsuite/gcc.target/arm/mve/pr122858.c
>> new file mode 100644
>> index 00000000000..d77d395d154
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/mve/pr122858.c
>> @@ -0,0 +1,40 @@
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target arm_mve_hw } */
>> +/* { dg-options "-O2" } */
>> +/* { dg-add-options arm_v8_1m_mve } */
>> +
>> +extern void abort (void);
>> +
>> +__attribute__ ((noipa))
>> +long long ashrl_fn (long long a)
>> +{
>> + long long c;
>> +
>> + c = a >> 31;
>> + c += a;
>> + return c;
>> +}
>> +
>> +__attribute__ ((noipa))
>> +long long ashll_fn (long long a)
>> +{
>> + long long c;
>> +
>> + c = a << 33;
>> + c += a;
>> + return c;
>> +}
>> +
>> +int main(void)
>> +{
>> + long long var1 = 1;
>> + long long var2 = ashll_fn (var1);
>> + if (var2 != 0x200000001)
>> + abort ();
>> +
>> + var2 = ashrl_fn (var2);
>> + if (var2 != 0x200000005)
>> + abort ();
>> +
>> + return 0;
>> +}
>
