Hi Richard, On 2026-06-10 12:34, Richard Earnshaw (foss) wrote:
On 24/03/2026 15:00, Torbjorn SVENSSON wrote:On 2026-03-06 15:40, Richard Earnshaw (foss) wrote:On 23/02/2026 10:24, Torbjörn SVENSSON wrote:With this patch, both variations are accepted. In trunk, only the first one is used unless -fno-fuse-ops-with-volatile-access is given, so I sent a separate patch for trunk in https://gcc.gnu.org/pipermail/gcc-patches/2026-February/709021.html Also worth noting that the tests still fail if -mtune=cortex-m7 is used. On releases/gcc-15, Cortex-M7 produces this assembler instead: stacktest1: sub sp, sp, #8 strh r0, [sp, #6] @ __bf16 add r3, sp, #6 ldrh r0, [sp, #6] @ __bf16 add sp, sp, #8 bx lr On trunk, this does not happen. Ok for releases/gcc-15? -- Some targets generate sub-optimal assembly where an intermediate register is used for the stack offset, while others avoids the extra register and produce slightly smaller code. Adjust expected assembler to match both. Example assembler for thumb/arch=armv6s-m/tune=cortex-m0/float-abi=soft stacktest1: sub sp, sp, #8 strh r0, [sp, #6] @ __bf16 ldrh r0, [sp, #6] @ __bf16 add sp, sp, #8 bx lrIf GCC can now generate this, thenExample assembler for thumb/cpu=cortex-m0/float-abi=soft stacktest1: sub sp, sp, #8 add r3, sp, #6 strh r0, [r3] @ __bf16 ldrh r0, [sp, #6] @ __bf16 add sp, sp, #8 bx lrGenerating this now seems like a bug and we need to at least find out why.I'm not sure if this is a bug or not, but one thing that I am sure of is that the -mtune=cortex-m0 appears to "survive" the -mcpu=unset and thus will still have an impact after the arch has been changed. Thus, I think the problem is only related to the test cases and not a real life issue. On top of this, in GCC-16, the extra register is optimized away, so the problem is masked there.Which is why I would not want the sub-optimal sequence to be a permitted result on trunk. We shouldn't allow it to regress back to a separate add.
The tests in releases/gcc-16 and trunk rejects the usage of anything but strh with an offset to sp, i.e., it does not allow r3. Based on this, I think we are safe for these tests. Then, if -fno-fuse-ops-with-volatile-access is used, we could still end up with the add instruction followed by using that as the argument to strh.
In real life, I do not see that anyone would specify both -march and -mtune that would not be compatible.Seperate tune and arch options came about precisely because we needed to build for one architecture, but tune for an implementation with a later architecture. Back in the armv4 days many people had Acorn machines with either an arm7 or a StrongArm CPU. We wanted code that would run on both, but that over time would work best on the StrongArm which rappidly supplanted the original arm7 based machines. I imlemented support for -march=armv3 -mtune=strongarm to cover exactly that scenario. Admittedly, it was never expected that this would support incompatible permutations, like -march=armv7-a -mtune=cortex-m0 (ie armv6m) -- that bifurcation of the architecture didn't become relevant until cortex-m3 came out about 10 years later; but tuning only affects how we pick from the available set of instructions, not which instructions are available to pick so it should still always generate code that will run even if the results make no sense from a performance perspective.
I understand this need, but then the values of -mtune and -march are compatible, not like cortex-m with armv7-a as you used as an example above. Anyhow, this feels out of context for this patch.
Note, the cortex-a55 tune will case the r3 register to be used, just like when I'm running the test suite with cpu=cortex-m0.gcc/testsuite/ChangeLog: * gcc.target/arm/bfloat16_scalar_1_2.c: Adjust assembler to match compiler. * gcc.target/arm/bfloat16_scalar_2_2.c: * gcc.target/arm/bfloat16_scalar_3_2.c: * gcc.target/arm/bfloat16_simd_1_2.c: * gcc.target/arm/bfloat16_simd_2_2.c: * gcc.target/arm/bfloat16_simd_3_2.c:So on that basis, I don't think this is the right way to fix this, at least, not without clarity as to the reason for the code difference.Based on above statements, do you still feel this is wrong solution for gcc-15? Keep in mind that gcc-16 has a different solution.My main concern is the testsuite for the latest release; but even there we have many compiler options which can perturb the code in ways that the scan tests simply can't ever work reliably (stack canaries, forced frame pointers, etc). I'm inclining towards saying that running the testsuite with a -mtune in the test configuration is simply not supported - if we want to test things like that, we would need specific tests for those options.
I tend to agree, but I do not see any way to get the test results stable for these tests. I've opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125744 as a followup ticket regarding the root cause why `add r3, sp, #6` might be picked in certain conditions. Let me know how you want to proceed with this patch. Maybe we should simply accept how GCC15 is and drop this patch? Kind regards, Torbjörn
R.Kind regards, TorbjörnR.--- gcc/testsuite/gcc.target/arm/bfloat16_scalar_1_2.c | 10 ++++++++-- gcc/testsuite/gcc.target/arm/bfloat16_scalar_2_2.c | 10 ++++++++-- gcc/testsuite/gcc.target/arm/bfloat16_scalar_3_2.c | 10 ++++++++-- gcc/testsuite/gcc.target/arm/bfloat16_simd_1_2.c | 10 ++++++++-- gcc/testsuite/gcc.target/arm/bfloat16_simd_2_2.c | 10 ++++++++-- gcc/testsuite/gcc.target/arm/bfloat16_simd_3_2.c | 10 ++++++++-- 6 files changed, 48 insertions(+), 12 deletions(-) diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_1_2.c b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_1_2.c index 079814ef337..fb3f11c39f3 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_1_2.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_1_2.c @@ -9,8 +9,14 @@ /* **stacktest1: ** ... -** strh r[0-9]+, \[r[0-9]+\] @ __bf16 -** ldrh r[0-9]+, \[sp, #[0-9]+\] @ __bf16 +**( +** strh r[0-9]+, (\[sp, #[0-9]+\]) @ __bf16 +** ldrh r[0-9]+, \1 @ __bf16 +**| +** add (r[0-9]+), (sp, #[0-9]+) +** strh r[0-9]+, \[\2] @ __bf16 +** ldrh r[0-9]+, \[\3\] @ __bf16 +**) ** ... ** bx lr */ diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_2_2.c b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_2_2.c index fc252b94edc..b08aeeed56c 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_2_2.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_2_2.c @@ -12,8 +12,14 @@ /* **stacktest1: ** ... -** strh r[0-9]+, \[r[0-9]+\] @ __bf16 -** ldrh r[0-9]+, \[sp, #[0-9]+\] @ __bf16 +**( +** strh r[0-9]+, (\[sp, #[0-9]+\]) @ __bf16 +** ldrh r[0-9]+, \1 @ __bf16 +**| +** add (r[0-9]+), (sp, #[0-9]+) +** strh r[0-9]+, \[\2] @ __bf16 +** ldrh r[0-9]+, \[\3\] @ __bf16 +**) ** ... ** bx lr */ diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_3_2.c b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_3_2.c index 079814ef337..fb3f11c39f3 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_3_2.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_3_2.c @@ -9,8 +9,14 @@ /* **stacktest1: ** ... -** strh r[0-9]+, \[r[0-9]+\] @ __bf16 -** ldrh r[0-9]+, \[sp, #[0-9]+\] @ __bf16 +**( +** strh r[0-9]+, (\[sp, #[0-9]+\]) @ __bf16 +** ldrh r[0-9]+, \1 @ __bf16 +**| +** add (r[0-9]+), (sp, #[0-9]+) +** strh r[0-9]+, \[\2] @ __bf16 +** ldrh r[0-9]+, \[\3\] @ __bf16 +**) ** ... ** bx lr */ diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_simd_1_2.c b/gcc/testsuite/gcc.target/arm/bfloat16_simd_1_2.c index 299bd60086b..58f75a73c05 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_simd_1_2.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_simd_1_2.c @@ -11,8 +11,14 @@ /* **stacktest1: ** ... -** strh r[0-9]+, \[r[0-9]+\] @ __bf16 -** ldrh r[0-9]+, \[sp, #[0-9]+\] @ __bf16 +**( +** strh r[0-9]+, (\[sp, #[0-9]+\]) @ __bf16 +** ldrh r[0-9]+, \1 @ __bf16 +**| +** add (r[0-9]+), (sp, #[0-9]+) +** strh r[0-9]+, \[\2] @ __bf16 +** ldrh r[0-9]+, \[\3\] @ __bf16 +**) ** ... ** bx lr */ diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_simd_2_2.c b/gcc/testsuite/gcc.target/arm/bfloat16_simd_2_2.c index 9b1ff278041..ca7473acfac 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_simd_2_2.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_simd_2_2.c @@ -14,8 +14,14 @@ /* **stacktest1: ** ... -** strh r[0-9]+, \[r[0-9]+\] @ __bf16 -** ldrh r[0-9]+, \[sp, #[0-9]+\] @ __bf16 +**( +** strh r[0-9]+, (\[sp, #[0-9]+\]) @ __bf16 +** ldrh r[0-9]+, \1 @ __bf16 +**| +** add (r[0-9]+), (sp, #[0-9]+) +** strh r[0-9]+, \[\2] @ __bf16 +** ldrh r[0-9]+, \[\3\] @ __bf16 +**) ** ... ** bx lr */ diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_simd_3_2.c b/gcc/testsuite/gcc.target/arm/bfloat16_simd_3_2.c index ec9f4a0d690..63f54e32c55 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_simd_3_2.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_simd_3_2.c @@ -11,8 +11,14 @@ /* **stacktest1: ** ... -** strh r[0-9]+, \[r[0-9]+\] @ __bf16 -** ldrh r[0-9]+, \[sp, #[0-9]+\] @ __bf16 +**( +** strh r[0-9]+, (\[sp, #[0-9]+\]) @ __bf16 +** ldrh r[0-9]+, \1 @ __bf16 +**| +** add (r[0-9]+), (sp, #[0-9]+) +** strh r[0-9]+, \[\2] @ __bf16 +** ldrh r[0-9]+, \[\3\] @ __bf16 +**) ** ... ** bx lr */
