On 14/07/2026 13:31, Richard Earnshaw (foss) wrote:
> On 13/07/2026 16:00, Torbjörn SVENSSON wrote:
>> Ok for trunk, releases/gcc-16 and releases/gcc-15?
>>
>> --
>>
>> In r15-1579-g792f97b44ffc5e, a new late-combine pass was introduced that
>> changes the generated assembler from
>> ldr r3, [sp]
>> vdup.32 q8, r3
>> to
>> vld1.32 {d16[], d17[]}, [sp]
>> with -mthumb and from
>> vmov r3, s0 @ int
>> vdup.32 q8, r3
>> to
>> vdup.32 q8, d0[0]
>> with -marm.
>>
>> In both modes, the check
>>
>> /* { dg-final { scan-assembler-times {vdup.32\tq[0-9]+, r[0-9]+} 4 } } */
>>
>> will no longer match. Avoid the fail state by disabling the late-combine
>> pass.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR testsuite/124043
>> * gcc.target/arm/crypto-vsha1cq_u32.c: Add
>> -fno-late-combine-instructions to test.
>> * gcc.target/arm/crypto-vsha1h_u32.c: Likewise.
>> * gcc.target/arm/crypto-vsha1mq_u32.c: Likewise.
>> * gcc.target/arm/crypto-vsha1pq_u32.c: Likewise.
>>
>> Signed-off-by: Torbjörn SVENSSON <[email protected]>
>> ---
>> gcc/testsuite/gcc.target/arm/crypto-vsha1cq_u32.c | 2 +-
>> gcc/testsuite/gcc.target/arm/crypto-vsha1h_u32.c | 2 +-
>> gcc/testsuite/gcc.target/arm/crypto-vsha1mq_u32.c | 2 +-
>> gcc/testsuite/gcc.target/arm/crypto-vsha1pq_u32.c | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/crypto-vsha1cq_u32.c
>> b/gcc/testsuite/gcc.target/arm/crypto-vsha1cq_u32.c
>> index 0cadd19c4dc..8c26ac204d2 100644
>> --- a/gcc/testsuite/gcc.target/arm/crypto-vsha1cq_u32.c
>> +++ b/gcc/testsuite/gcc.target/arm/crypto-vsha1cq_u32.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-effective-target arm_crypto_ok } */
>> /* { dg-add-options arm_crypto } */
>> -/* { dg-additional-options "-O3" } */
>> +/* { dg-additional-options "-O3 -fno-late-combine-instructions" } */
>>
>> #include "arm_neon.h"
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/crypto-vsha1h_u32.c
>> b/gcc/testsuite/gcc.target/arm/crypto-vsha1h_u32.c
>> index 33af705c59e..c8a47f16991 100644
>> --- a/gcc/testsuite/gcc.target/arm/crypto-vsha1h_u32.c
>> +++ b/gcc/testsuite/gcc.target/arm/crypto-vsha1h_u32.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-effective-target arm_crypto_ok } */
>> /* { dg-add-options arm_crypto } */
>> -/* { dg-additional-options "-O3" } */
>> +/* { dg-additional-options "-O3 -fno-late-combine-instructions" } */
>>
>> #include "arm_neon.h"
>>
>
> Looking at the assembler output for this test with -march=armv8-a+crypto
> -mfloat-abi=hard, I see
>
> for thumb:
>
> foo_lane0:
> sub sp, sp, #16
> vst1.64 {d0-d1}, [sp:64]
> vld1.32 {d16[], d17[]}, [sp]
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> add sp, sp, #16
> bx lr
> foo_lane1:
> vmov.32 r3, d0[1]
> vdup.32 q8, r3
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
> foo_lane2:
> vmov.32 r3, d1[0]
> vdup.32 q8, r3
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
> foo_lane3:
> vmov.32 r3, d1[1]
> vdup.32 q8, r3
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
>
> and for arm:
>
> foo_lane0:
> vdup.32 q8, d0[0]
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
> foo_lane1:
> vmov.32 r3, d0[1]
> vdup.32 q8, r3
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
> foo_lane2:
> vmov.32 r3, d1[0]
> vdup.32 q8, r3
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
> foo_lane3:
> vmov.32 r3, d1[1]
> vdup.32 q8, r3
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
>
> I really don't understand right now why all these patterns are not
> simplifying to the general format of the arm implementation of foo_lane0;
> there's no need to copy the value into a core register, and there's even less
> need to spill values onto the stack, as is done in the thumb variant for
> lane0.
So the starting point for this generating poor code is that we lack patterns
for lane dups that read from 128-bit vector modes (we only have patterns that
read from 64-bit modes in general). There's no fundamental reason why we can't
have such patterns, but I think it's simply because ACLE doesn't define
intrinsics for this.
R.
>
> For vfp/neon the Arm and Thumb instruction sets are practially identical, so
> there really is no reason to have different code-gen here:
>
> foo_lane0:
> vdup.32 q8, d0[0]
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
> foo_lane1:
> vdup.32 q8, d0[1]
> sha1c.32 q1, q8, q2
> vmov q0, q1 @ v4si
> bx lr
>
> assembles just fine in both Arm and Thumb modes (the other two are the same,
> except using d1).
>
> So I think we should xfail these tests for now, and create a PR for the
> cleanup to the codegen. The tests should
> really be converted to scan-function-body tests as well, then it becomes
> clearer what is expected; but that can wait for the code to fixed first.
>
> R.
>
>> diff --git a/gcc/testsuite/gcc.target/arm/crypto-vsha1mq_u32.c
>> b/gcc/testsuite/gcc.target/arm/crypto-vsha1mq_u32.c
>> index bdd1c4f3315..a5a0103049b 100644
>> --- a/gcc/testsuite/gcc.target/arm/crypto-vsha1mq_u32.c
>> +++ b/gcc/testsuite/gcc.target/arm/crypto-vsha1mq_u32.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-effective-target arm_crypto_ok } */
>> /* { dg-add-options arm_crypto } */
>> -/* { dg-additional-options "-O3" } */
>> +/* { dg-additional-options "-O3 -fno-late-combine-instructions" } */
>>
>> #include "arm_neon.h"
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/crypto-vsha1pq_u32.c
>> b/gcc/testsuite/gcc.target/arm/crypto-vsha1pq_u32.c
>> index d48a07c6fa4..9ef962c28ae 100644
>> --- a/gcc/testsuite/gcc.target/arm/crypto-vsha1pq_u32.c
>> +++ b/gcc/testsuite/gcc.target/arm/crypto-vsha1pq_u32.c
>> @@ -1,7 +1,7 @@
>> /* { dg-do compile } */
>> /* { dg-require-effective-target arm_crypto_ok } */
>> /* { dg-add-options arm_crypto } */
>> -/* { dg-additional-options "-O3" } */
>> +/* { dg-additional-options "-O3 -fno-late-combine-instructions" } */
>>
>> #include "arm_neon.h"
>>
>
