Re: [committed 03/22] arm: testsuite: avoid hard-float ABI incompatibility with -march

[Christophe Lyon <christophe.lyon@linaro.org>]

On Mon, 20 Nov 2023 at 15:39, Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
>
>
> On 20/11/2023 14:24, Christophe Lyon wrote:
> > On Mon, 20 Nov 2023 at 14:58, Richard Earnshaw
> > <Richard.Earnshaw@foss.arm.com> wrote:
> >>
> >>
> >>
> >> On 20/11/2023 13:36, Christophe Lyon wrote:
> >>> On Mon, 20 Nov 2023 at 13:44, Richard Earnshaw
> >>> <Richard.Earnshaw@foss.arm.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 20/11/2023 10:23, Christophe Lyon wrote:
> >>>>> Hi Richard,
> >>>>>
> >>>>> On Mon, 13 Nov 2023 at 15:28, Richard Earnshaw <rearnsha@arm.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>> A number of tests in the gcc testsuite, especially for arm-specific
> >>>>>> targets, add various flags to control the architecture.  These run
> >>>>>> into problems when the compiler is configured with -mfpu=auto if the
> >>>>>> new architecture lacks an architectural feature that implies we have
> >>>>>> floating-point instructions.
> >>>>>>
> >>>>>> The testsuite makes this worse as it falls foul of this requirement in
> >>>>>> the base architecture strings provided by target-supports.exp.
> >>>>>>
> >>>>>> To fix this we add "+fp", or something equivalent to this, to all the
> >>>>>> base architecture specifications.  The feature will be ignored if the
> >>>>>> float ABI is set to soft.
> >>>>>>
> >>>>>> gcc/testsuite:
> >>>>>>
> >>>>>>           * lib/target-supports.exp (check_effective_target_arm_arch_FUNC_ok):
> >>>>>>           Add base FPU specifications to all architectures that can support
> >>>>>>           one.
> >>>>>> ---
> >>>>>>    gcc/testsuite/lib/target-supports.exp | 50 +++++++++++++--------------
> >>>>>>    1 file changed, 25 insertions(+), 25 deletions(-)
> >>>>>>
> >>>>>
> >>>>> Our CI has detected some regressions with this patch, in particular
> >>>>> when testing for cortex-m55:
> >>>>>
> >>>>> with
> >>>>> -mthumb/-march=armv8.1-m.main+mve.fp+fp.dp/-mtune=cortex-m55/-mfloat-abi=hard/-mfpu=auto
> >>>>> and GCC configured with --disable-multilib --with-mode=thumb
> >>>>> --with-arch=armv8.1-m.main+mve.fp+fp.dp --with-float=hard
> >>>>>
> >>>>> you can see our logs here:
> >>>>> https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m55_hard_eabi-build/209/artifact/artifacts/00-sumfiles/ <https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m55_hard_eabi-build/209/artifact/artifacts/00-sumfiles/>
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Christophe
> >>>>
> >>>> What exactly am I supposed to be looking at here?  I see no description
> >>>> of what those logs represent.  If they are supposed to be before and
> >>>> after, then why does the after only run a tiny fraction of the testsuite
> >>>> (Running gcc.git~master/gcc/testsuite/gcc.target/arm/arm.exp ...
> >>>> Running gcc.git~master/gcc/testsuite/gcc.target/arm/cmse/cmse.exp ...
> >>>> Running gcc.git~master/gcc/testsuite/gcc.target/arm/lto/lto.exp ...)
> >>>>
> >>>> The logs give no clue as to why the reminder of the testsuite wasn't run.
> >>>>
> >>>> Please don't make me guess.
> >>>>
> >>>
> >>> Here is a summary with the list of regressions:
> >>> https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m55_hard_eabi-build/209/artifact/artifacts/notify/regressions.sum/*view*/
> >>
> >> OK, that's much more useful.  But how was I supposed to know that link
> >> existed?
> >>
> > The full notification email contains a lot of information, with
> > several pointers to our Jenkins artifacts.
> > The notification email is not yet automatically sent to contributors
> > because we are still polishing, and I thought I'd save you some time
> > by just sending the useful links.
> >
> > Looks like it's time to send those automatically too.
> >
> >>>
> >>> I thought you'd be able to find your way in the logs above, the .0
> >>> files contain the logs of the initial full testsuite run, and .1 ones
> >>> contain the logs of the second run of the testsuite, restricted to the
> >>> .exp files where we detected regressions. So looking at gcc.log.1.xz
> >>> will give you details of the regressions shown in the link above.
> >>
> >> There's nothing in the page you sent me to that gives any clue as to how
> >> to read the logs there.  So my assumption was that the .0 was a before
> >> run and .1 an after.  Please, if you're going to direct people to the
> >> log files, provide some way for them to understand what the log files show.
> >>
> >> Now, to the specific issues:
> >>
> >> Running gcc:gcc.target/arm/arm.exp ...
> >> FAIL: gcc.target/arm/attr_thumb-static2.c (test for excess errors)
> >> UNRESOLVED: gcc.target/arm/attr_thumb-static2.c scan-assembler-times blx 2
> >>
> >> This was fixed with "arm: testsuite: avoid problems with -mfpu=auto in
> >> attr_thumb-static2.c", which is a later patch in the series (patch 6).
> >>
> >> I don't think it's useful to try to regression test each individual
> >> patch, it wasn't practical to try to get every patch into order in the
> >> series (it would have made for a lot of churn on some files, especially
> >> target-supports.exp), so only a fully before and a fully after run is
> >> useful.  If there are issues once the whole series has been applied,
> >> then that is much more interesting.
> >>
> >
> > I looked at this in more detail.
> > That specific bisection build was triggered because we detected
> > regressions, after the full series was committed.
> > What happens is that at the first bad commit (this one) there were
> > more regressions than after the full series was committed.
> >
> > So, the extract of
> > https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m55_hard_eabi-build/209/artifact/artifacts/notify/regressions.sum/*view*/
> > which remains valid on current trunk is:
> > Running gcc:gcc.target/arm/cmse/cmse.exp ...
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O0   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O1   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O2   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O3 -g   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -Os   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O0   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O1   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O2   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O3 -g   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/hard/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -Os   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/soft/cmse-5.c
> > -march=armv8-m.main+fp -mthumb -mfloat-abi=soft  -O0   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/soft/cmse-5.c
> > -march=armv8-m.main+fp -mthumb -mfloat-abi=soft  -O1   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/soft/cmse-5.c
> > -march=armv8-m.main+fp -mthumb -mfloat-abi=soft  -O2   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/soft/cmse-5.c
> > -march=armv8-m.main+fp -mthumb -mfloat-abi=soft  -O3 -g
> > scan-assembler msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/soft/cmse-5.c
> > -march=armv8-m.main+fp -mthumb -mfloat-abi=soft  -Os   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O0   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O1   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O2   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O3 -g   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp-sp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -Os   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O0   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O1   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O2   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -O3 -g   scan-assembler
> > msr\tAPSR_nzcvqg, lr
> > FAIL: gcc.target/arm/cmse/mainline/8m/softfp/cmse-5.c
> > -march=armv8-m.main+fp -mthumb  -Os   scan-assembler msr\tAPSR_nzcvqg,
> > lr
> >
> >> R.
>
> The compiled output contains (at least for the case I tried -mthumb
> -march=armv8.1-m.main+mve.fp+fp.dp -mtune=cortex-m55 -mfloat-abi=hard
> -mfpu=auto   -fdiagnostics-plain-output  -march=armv8-m.base -mthumb
> -mfloat-abi=soft -O1 -mcmse -ffat-lto-objects -fno-ident -S  -o cmse-2.s):
>
> msr     APSR_nzcvq, r1
>
> So this will never match the expected pattern, which is looking for 'lr'
> not 'r1'.  Are you sure these tests were running before?
>

No, your patch enabled them.

I already noticed/reported a long time ago that cmse.exp was not
executed by all the configurations we currently run and I was
surprised to see errors in my manual runs with more specific
configurations.

Good to see it enabled now.


> R.