On Fri, 27 Nov 2020 at 02:03, Stam Markianos-Wright
<stam.markianos-wri...@arm.com> wrote:
>
> 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?
I guess that's OK for me.
We can probably increase the timeout value if needed.
>
> 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.
>
