Re: [committed obvious][arm] Add test that was missing from old commit [PR91816]

[Stam Markianos-Wright via Gcc-patches]

On 26/11/2020 09:01, Christophe Lyon wrote:
On Wed, 25 Nov 2020 at 14:24, Stam Markianos-Wright via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:

Hi all,

A while back I submitted GCC10 commit:

   44f77a6dea2f312ee1743f3dde465c1b8453ee13

for PR91816.

Turns out I was an idiot and forgot to include the test in the actual
git commit, even my entire patch had been approved.

Tested that the test still passes on a cross arm-none-eabi and also in a
Cortex A-15 bootstrap with no regressions.

Submitting this as Obvious to gcc-11 and backporting to gcc-10.


Hi,

This new test fails when forcing -mcpu=cortex-m3/4/5/7/33:
FAIL: gcc.target/arm/pr91816.c scan-assembler-times beq\\t.L[0-9] 2
FAIL: gcc.target/arm/pr91816.c scan-assembler-times beq\\t.Lbcond[0-9] 1
FAIL: gcc.target/arm/pr91816.c scan-assembler-times bne\\t.L[0-9] 2
FAIL: gcc.target/arm/pr91816.c scan-assembler-times bne\\t.Lbcond[0-9] 1

I didn't check manually what is generated, can you have a look?


Oh wow thank you for spotting this!

It looks like the A class target that I had tested had a tendency to 
emit a movw/movt pair, whereas these M class targets would emit a single 
ldr. This resulted in an overall shorter jump for these targets that did 
not trigger the new far-branch code.

The test passes after... doubling it's own size:



 #define HW3    HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2 HW2
 #define HW4    HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3 HW3
 #define HW5    HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4 HW4
+#define HW6    HW5 HW5

 __attribute__((noinline,noclone)) void f1 (int a)
 {
@@ -25,7 +26,7 @@ __attribute__((noinline,noclone)) void f2 (int a)

 __attribute__((noinline,noclone)) void f3 (int a)
 {
-  if (a) { HW5 }
+  if (a) { HW6 }
 }

 __attribute__((noinline,noclone)) void f4 (int a)
@@ -41,7 +42,7 @@ __attribute__((noinline,noclone)) void f5 (int a)

 __attribute__((noinline,noclone)) void f6 (int a)
 {
-  if (a == 1) { HW5 }
+  if (a == 1) { HW6 }
 }

But this does effectively double the compilation time of an already 
quite large test. Would that be ok?

Overall this is the edge case testing that the compiler behaves 
correctly with a branch in huge compilation unit, so it would be nice to 
have test coverage of it on as many targets as possible... but also 
kinda rare.

Hope this helps!

Cheers,
Stam



Thanks,

Christophe




Thanks,
Stam Markianos-Wright

gcc/testsuite/ChangeLog:
         PR target/91816
         * gcc.target/arm/pr91816.c: New test.