diff mbox series

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

Message ID 20231113142658.69039-4-rearnsha@arm.com
State New
Headers show
Series arm: testsuite: clean up some architecture-specific tests | expand

Commit Message

Richard Earnshaw Nov. 13, 2023, 2:26 p.m. UTC
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(-)

Comments

Christophe Lyon Nov. 20, 2023, 10:23 a.m. UTC | #1
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/

Thanks,

Christophe
Richard Earnshaw Nov. 20, 2023, 12:44 p.m. UTC | #2
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.

R.
Christophe Lyon Nov. 20, 2023, 1:36 p.m. UTC | #3
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*/

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.

Christophe

> R.
Richard Earnshaw Nov. 20, 2023, 1:58 p.m. UTC | #4
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?

> 
> 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.

R.
Christophe Lyon Nov. 20, 2023, 2:24 p.m. UTC | #5
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.
Richard Earnshaw Nov. 20, 2023, 2:39 p.m. UTC | #6
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?

R.
Richard Earnshaw Nov. 20, 2023, 2:49 p.m. UTC | #7
On 20/11/2023 14:39, Richard Earnshaw 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?
> 
> R.

Sorry, ignore that, I was looking at the wrong test.

R.
Christophe Lyon Nov. 20, 2023, 2:50 p.m. UTC | #8
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.
Richard Earnshaw Nov. 20, 2023, 3:58 p.m. UTC | #9
On 20/11/2023 14:50, Christophe Lyon wrote:
> 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.

[TLDR]

I was initially confused about why this test had suddenly started to 
run, but it turns out it's because cmse.exp has the gating operation, 
not the test itself.

The problem in this case is because the specific subtest is checking for 
the DSP extension.  But we have a command line of

  -mthumb -march=armv8.1-m.main+mve.fp+fp.dp -mtune=cortex-m55 \
  -mfloat-abi=hard -mfpu=auto -fdiagnostics-plain-output \
  -march=armv8-m.main+fp -mthumb -mfloat-abi=soft -O1 -mcmse \
  -mfloat-abi=soft

When compiling the test, but a command line of

  -mthumb -march=armv8.1-m.main+mve.fp+fp.dp -mtune=cortex-m55 \
  -mfloat-abi=hard -mfpu=auto

when checking for the DSP extension.

The former is built up from

  -mthumb -march=armv8.1-m.main+mve.fp+fp.dp -mtune=cortex-m55 \
  -mfloat-abi=hard -mfpu=auto

for the testsuite run

  -fdiagnostics-plain-output

from the general framework

  -march=armv8-m.main+fp -mthumb

from the cmse.exp script [add_options_for...]

  -mfloat-abi=soft

from the framework - variant check

and finally

  -O1 -mcmse -mfloat-abi=soft

from the dg-options in the test.

So we're going to get a mismatch in features here which is not trivial 
to work around.  The problem in this particular case stems from the fact 
that the DSP extension is mandatory if MVE is present, but optional 
otherwise and the option combinations imply it via +mve.fp.  We then 
override the architecture with a non-MVE variant and end up not having 
DSP.  Ultimately, therefore, we compile the test without DSP, but then 
check for DSP with a set of architecture flags that has DSP.

Given the way this is all put together, I think you'd see the same 
problem if you ran the test with

  -mthumb -march=armv8.1-m.main+fp+dsp -mfloat-abi=softfp

I don't think there's a trivial way to work around the inconsistent 
options problem without weakening the test overall (by just testing for 
"APSR_nzcvqg?").  So perhaps the best solution is to do that, but then 
add some additional tests that are outside this base CMSE behaviour that 
test the APSR register more explicitly.

I'll give this some thought.

R.
diff mbox series

Patch

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index ee173b9fb6b..7d83bd8740f 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -5408,36 +5408,36 @@  foreach { armfunc armflag armdefs } {
 	v5t "-march=armv5t -mfloat-abi=softfp" __ARM_ARCH_5T__
 	v5t_arm "-march=armv5t -marm" __ARM_ARCH_5T__
 	v5t_thumb "-march=armv5t -mthumb -mfloat-abi=softfp" __ARM_ARCH_5T__
-	v5te "-march=armv5te -mfloat-abi=softfp" __ARM_ARCH_5TE__
-	v5te_arm "-march=armv5te -marm" __ARM_ARCH_5TE__
-	v5te_thumb "-march=armv5te -mthumb -mfloat-abi=softfp" __ARM_ARCH_5TE__
-	v6 "-march=armv6 -mfloat-abi=softfp" __ARM_ARCH_6__
-	v6_arm "-march=armv6 -marm" __ARM_ARCH_6__
-	v6_thumb "-march=armv6 -mthumb -mfloat-abi=softfp" __ARM_ARCH_6__
-	v6k "-march=armv6k -mfloat-abi=softfp" __ARM_ARCH_6K__
-	v6k_arm "-march=armv6k -marm" __ARM_ARCH_6K__
-	v6k_thumb "-march=armv6k -mthumb -mfloat-abi=softfp" __ARM_ARCH_6K__
-	v6t2 "-march=armv6t2" __ARM_ARCH_6T2__
-	v6z "-march=armv6z -mfloat-abi=softfp" __ARM_ARCH_6Z__
-	v6z_arm "-march=armv6z -marm" __ARM_ARCH_6Z__
-	v6z_thumb "-march=armv6z -mthumb -mfloat-abi=softfp" __ARM_ARCH_6Z__
+	v5te "-march=armv5te+fp -mfloat-abi=softfp" __ARM_ARCH_5TE__
+	v5te_arm "-march=armv5te+fp -marm" __ARM_ARCH_5TE__
+	v5te_thumb "-march=armv5te+fp -mthumb -mfloat-abi=softfp" __ARM_ARCH_5TE__
+	v6 "-march=armv6+fp -mfloat-abi=softfp" __ARM_ARCH_6__
+	v6_arm "-march=armv6+fp -marm" __ARM_ARCH_6__
+	v6_thumb "-march=armv6+fp -mthumb -mfloat-abi=softfp" __ARM_ARCH_6__
+	v6k "-march=armv6k+fp -mfloat-abi=softfp" __ARM_ARCH_6K__
+	v6k_arm "-march=armv6k+fp -marm" __ARM_ARCH_6K__
+	v6k_thumb "-march=armv6k+fp -mthumb -mfloat-abi=softfp" __ARM_ARCH_6K__
+	v6t2 "-march=armv6t2+fp" __ARM_ARCH_6T2__
+	v6z "-march=armv6z+fp -mfloat-abi=softfp" __ARM_ARCH_6Z__
+	v6z_arm "-march=armv6z+fp -marm" __ARM_ARCH_6Z__
+	v6z_thumb "-march=armv6z+fp -mthumb -mfloat-abi=softfp" __ARM_ARCH_6Z__
 	v6m "-march=armv6-m -mthumb -mfloat-abi=soft" __ARM_ARCH_6M__
-	v7a "-march=armv7-a" __ARM_ARCH_7A__
-	v7r "-march=armv7-r" __ARM_ARCH_7R__
+	v7a "-march=armv7-a+fp" __ARM_ARCH_7A__
+	v7r "-march=armv7-r+fp" __ARM_ARCH_7R__
 	v7m "-march=armv7-m -mthumb" __ARM_ARCH_7M__
-	v7em "-march=armv7e-m -mthumb" __ARM_ARCH_7EM__
-	v7ve "-march=armv7ve -marm"
+	v7em "-march=armv7e-m+fp -mthumb" __ARM_ARCH_7EM__
+	v7ve "-march=armv7ve+fp -marm"
 		"__ARM_ARCH_7A__ && __ARM_FEATURE_IDIV"
-	v8a "-march=armv8-a" __ARM_ARCH_8A__
-	v8a_hard "-march=armv8-a -mfpu=neon-fp-armv8 -mfloat-abi=hard" __ARM_ARCH_8A__
-	v8_1a "-march=armv8.1-a" __ARM_ARCH_8A__
-	v8_2a "-march=armv8.2-a" __ARM_ARCH_8A__
-	v8r "-march=armv8-r" __ARM_ARCH_8R__
+	v8a "-march=armv8-a+simd" __ARM_ARCH_8A__
+	v8a_hard "-march=armv8-a+simd -mfpu=auto -mfloat-abi=hard" __ARM_ARCH_8A__
+	v8_1a "-march=armv8.1-a+simd" __ARM_ARCH_8A__
+	v8_2a "-march=armv8.2-a+simd" __ARM_ARCH_8A__
+	v8r "-march=armv8-r+fp.sp" __ARM_ARCH_8R__
 	v8m_base "-march=armv8-m.base -mthumb -mfloat-abi=soft"
 		__ARM_ARCH_8M_BASE__
-	v8m_main "-march=armv8-m.main -mthumb" __ARM_ARCH_8M_MAIN__
-	v8_1m_main "-march=armv8.1-m.main -mthumb" __ARM_ARCH_8M_MAIN__
-	v9a "-march=armv9-a" __ARM_ARCH_9A__ } {
+	v8m_main "-march=armv8-m.main+fp -mthumb" __ARM_ARCH_8M_MAIN__
+	v8_1m_main "-march=armv8.1-m.main+fp -mthumb" __ARM_ARCH_8M_MAIN__
+	v9a "-march=armv9-a+simd" __ARM_ARCH_9A__ } {
     eval [string map [list FUNC $armfunc FLAG $armflag DEFS $armdefs ] {
 	proc check_effective_target_arm_arch_FUNC_ok { } {
 	    return [check_no_compiler_messages arm_arch_FUNC_ok assembly {