Re: [PATCH] [testsuite][arm] Fix cmse-15.c expected output
On 23/09/2022 09:43, Torbjörn SVENSSON via Gcc-patches wrote: The cmse-15.c testcase fails at -Os because ICF means that we generate secure3: b secure1 which is OK, but does not match the currently expected secure3: ... bx r[0-3] gcc/testsuite/ChangeLog: * gcc.target/arm/cmse/cmse-15.c: Align with -Os improvements. OK. R. Co-Authored-By: Yvan ROUX Signed-off-by: Torbjörn SVENSSON --- gcc/testsuite/gcc.target/arm/cmse/cmse-15.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c index b0fefe561a1..5188f1d697f 100644 --- a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c @@ -144,6 +144,8 @@ int secure2 (s_bar_ptr s_bar_p) **bx r[0-3] ** | **blx r[0-3] +** | +** b secure1 ** ) **... */
Re: [PATCH] [testsuite][arm] Fix cmse-15.c expected output
Hi Torbjörn, On 9/23/22 10:43, Torbjörn SVENSSON via Gcc-patches wrote: The cmse-15.c testcase fails at -Os because ICF means that we generate secure3: b secure1 which is OK, but does not match the currently expected secure3: ... bx r[0-3] gcc/testsuite/ChangeLog: * gcc.target/arm/cmse/cmse-15.c: Align with -Os improvements. Co-Authored-By: Yvan ROUX Signed-off-by: Torbjörn SVENSSON --- gcc/testsuite/gcc.target/arm/cmse/cmse-15.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c index b0fefe561a1..5188f1d697f 100644 --- a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c @@ -144,6 +144,8 @@ int secure2 (s_bar_ptr s_bar_p) **bx r[0-3] ** | **blx r[0-3] +** | +** b secure1 ** ) **... */ Yes, that looks OK to me, similar to a patch I made some time ago to the very same file, when ICF became smarter. It seems it is now able to notice that return (*s_bar_p) (); in secure3 () is equivalent to the same call in secure1 (). LGTM, but I'm not a maintainer. Thanks, Christophe
[PATCH] [testsuite][arm] Fix cmse-15.c expected output
The cmse-15.c testcase fails at -Os because ICF means that we generate secure3: b secure1 which is OK, but does not match the currently expected secure3: ... bx r[0-3] gcc/testsuite/ChangeLog: * gcc.target/arm/cmse/cmse-15.c: Align with -Os improvements. Co-Authored-By: Yvan ROUX Signed-off-by: Torbjörn SVENSSON --- gcc/testsuite/gcc.target/arm/cmse/cmse-15.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c index b0fefe561a1..5188f1d697f 100644 --- a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c @@ -144,6 +144,8 @@ int secure2 (s_bar_ptr s_bar_p) ** bx r[0-3] ** | ** blx r[0-3] +** | +** b secure1 ** ) ** ... */ -- 2.25.1
Re: [testsuite][arm] Fix cmse-15.c expected output
Christophe Lyon writes:
> On Wed, 8 Apr 2020 at 11:48, Richard Sandiford
> wrote:
>>
>> Christophe Lyon via Gcc-patches writes:
>> > Hi,
>> >
>> > While checking Martin's fix for PR ipa/94445, he made me realize that
>> > the cmse-15.c testcase still fails at -Os because ICF means that we
>> > generate
>> > nonsecure2:
>> > b nonsecure0
>> >
>> > which is OK, but does not match the currently expected
>> > nonsecure2:
>> > ...
>> > bl __gnu_cmse_nonsecure_call
>> >
>> > (see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html)
>> >
>> > The test has already different expectations for v8-M and v8.1-M.
>> >
>> > I've decided to try to use check-function-bodies to account for the
>> > different possibilities:
>> > - v8-M vs v8.1-M via two different prefixes
>> > - code generation variants (-0?) via multiple regexps
>> >
>> > I've tested that the test now passes with
>> > --target-board=-march=armv8-m.main
>> > and --target-board=-march=armv8.1-m.main.
>> >
>> > I feel this a bit too much of a burden for the purpose, maybe there's
>> > a better way of handling all these alternatives (in particular,
>> > there's a lot of duplication since the expected code for the secure*
>> > functions is the same for v8-M and v8.1-M).
>>
>> FWIW, an alternative is to give multiple versions with the same prefix
>> and use { target ... } to select between them. E.g.:
>>
>> /*
>> ** foo: { target a }
>> ** ...
>> */
>> /*
>> ** foo: { target { ! a } }
>> ** ...
>> */
>>
>
> Ha indeed, that makes it simpler. Thanks for the example, I hadn't
> fully understand how to use that scheme: I tried to add a third
> alternative (different prefix) with no selector for cases where no
> distinction was needed, but I realized that all alternatives need
> their full matching description.
>
> However, {target { ! a } } does not work as is, so the attached patch
> uses a non-greedy regexp to avoid trying "! a }" instead of "target {
> ! a }"
Oops. This part is definitely OK, thanks.
> If OK, maybe I should commit that as two separate patches?
I don't really feel qualified to review the substance of the arm part
of the patch, but given Andre's LGTM for that in the earlier version,
a rubber-stamp OK for that too. I agree separate commits might be
better.
Thanks,
Richard
Re: [testsuite][arm] Fix cmse-15.c expected output
On Wed, 8 Apr 2020 at 11:48, Richard Sandiford
wrote:
>
> Christophe Lyon via Gcc-patches writes:
> > Hi,
> >
> > While checking Martin's fix for PR ipa/94445, he made me realize that
> > the cmse-15.c testcase still fails at -Os because ICF means that we
> > generate
> > nonsecure2:
> > b nonsecure0
> >
> > which is OK, but does not match the currently expected
> > nonsecure2:
> > ...
> > bl __gnu_cmse_nonsecure_call
> >
> > (see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html)
> >
> > The test has already different expectations for v8-M and v8.1-M.
> >
> > I've decided to try to use check-function-bodies to account for the
> > different possibilities:
> > - v8-M vs v8.1-M via two different prefixes
> > - code generation variants (-0?) via multiple regexps
> >
> > I've tested that the test now passes with --target-board=-march=armv8-m.main
> > and --target-board=-march=armv8.1-m.main.
> >
> > I feel this a bit too much of a burden for the purpose, maybe there's
> > a better way of handling all these alternatives (in particular,
> > there's a lot of duplication since the expected code for the secure*
> > functions is the same for v8-M and v8.1-M).
>
> FWIW, an alternative is to give multiple versions with the same prefix
> and use { target ... } to select between them. E.g.:
>
> /*
> ** foo: { target a }
> ** ...
> */
> /*
> ** foo: { target { ! a } }
> ** ...
> */
>
Ha indeed, that makes it simpler. Thanks for the example, I hadn't
fully understand how to use that scheme: I tried to add a third
alternative (different prefix) with no selector for cases where no
distinction was needed, but I realized that all alternatives need
their full matching description.
However, {target { ! a } } does not work as is, so the attached patch
uses a non-greedy regexp to avoid trying "! a }" instead of "target {
! a }"
If OK, maybe I should commit that as two separate patches?
Thanks
Christophe
> Thanks,
> Richard
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
index 0e37b50..b0fefe5 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
@@ -1,5 +1,8 @@
/* { dg-do compile } */
/* { dg-options "-mcmse" } */
+/* ARMv8-M expectation with target { ! arm_cmse_clear_ok }. */
+/* ARMv8.1-M expectation with target arm_cmse_clear_ok. */
+/* { dg-final { check-function-bodies "**" "" "" } } */
int __attribute__ ((cmse_nonsecure_call)) (*ns_foo) (void);
int (*s_bar) (void);
@@ -11,67 +14,204 @@ typedef int s_bar_t (void);
typedef int __attribute__ ((cmse_nonsecure_call)) (* ns_foo_ptr) (void);
typedef int (*s_bar_ptr) (void);
+/*
+** nonsecure0: { target arm_cmse_clear_ok }
+** ...
+** blxns r[0-3]
+** ...
+*/
+/*
+** nonsecure0: { target { ! arm_cmse_clear_ok } }
+** ...
+** bl __gnu_cmse_nonsecure_call
+** ...
+*/
int nonsecure0 (ns_foo_t * ns_foo_p)
{
return ns_foo_p ();
}
+/*
+** nonsecure1: { target arm_cmse_clear_ok }
+** ...
+** blxns r[0-3]
+** ...
+*/
+/*
+** nonsecure1: { target { ! arm_cmse_clear_ok } }
+** ...
+** bl __gnu_cmse_nonsecure_call
+** ...
+*/
int nonsecure1 (ns_foo_t ** ns_foo_p)
{
return (*ns_foo_p) ();
}
+/*
+** nonsecure2: { target arm_cmse_clear_ok }
+** ...
+** (
+** blxns r[0-3]
+** |
+** b nonsecure0
+** )
+** ...
+*/
+/*
+** nonsecure2: { target { ! arm_cmse_clear_ok } }
+** ...
+** (
+** bl __gnu_cmse_nonsecure_call
+** |
+** b nonsecure0
+** )
+** ...
+*/
int nonsecure2 (ns_foo_ptr ns_foo_p)
{
return ns_foo_p ();
}
+
+/*
+** nonsecure3: { target arm_cmse_clear_ok }
+** ...
+** blxns r[0-3]
+** ...
+*/
+/*
+** nonsecure3: { target { ! arm_cmse_clear_ok } }
+** ...
+** bl __gnu_cmse_nonsecure_call
+** ...
+*/
int nonsecure3 (ns_foo_ptr * ns_foo_p)
{
return (*ns_foo_p) ();
}
+/*
+** secure0:
+** ...
+** (
+** bx r[0-3]
+** |
+** blx r[0-3]
+** )
+** ...
+*/
int secure0 (s_bar_t * s_bar_p)
{
return s_bar_p ();
}
+/*
+** secure1:
+** ...
+** (
+** bx r[0-3]
+** |
+** blx r[0-3]
+** )
+** ...
+*/
int secure1 (s_bar_t ** s_bar_p)
{
return (*s_bar_p) ();
}
+/*
+** secure2:
+** ...
+** (
+** bx r[0-3]
+** |
+** blx r[0-3]
+** |
+** b secure0
+** )
+** ...
+*/
int secure2 (s_bar_ptr s_bar_p)
{
return s_bar_p ();
}
+/*
+** secure3:
+** ...
+** (
+** bx r[0-3]
+** |
+** blx r[0-3]
+** )
+** ...
+*/
int secure3 (s_bar_ptr * s_bar_p)
{
return (*s_bar_p) ();
}
+/*
+** nonsecure4: { target arm_cmse_clear_ok }
+** ...
+** blxns r[0-3]
+** ...
+*/
+/*
+** nonsecure4: { target { ! arm_cmse_clear_ok } }
+** ...
+** bl __gnu_cmse_nonsecure_call
+** ...
+*/
int nonsecure4 (void)
{
return
Re: [testsuite][arm] Fix cmse-15.c expected output
Christophe Lyon via Gcc-patches writes:
> Hi,
>
> While checking Martin's fix for PR ipa/94445, he made me realize that
> the cmse-15.c testcase still fails at -Os because ICF means that we
> generate
> nonsecure2:
> b nonsecure0
>
> which is OK, but does not match the currently expected
> nonsecure2:
> ...
> bl __gnu_cmse_nonsecure_call
>
> (see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html)
>
> The test has already different expectations for v8-M and v8.1-M.
>
> I've decided to try to use check-function-bodies to account for the
> different possibilities:
> - v8-M vs v8.1-M via two different prefixes
> - code generation variants (-0?) via multiple regexps
>
> I've tested that the test now passes with --target-board=-march=armv8-m.main
> and --target-board=-march=armv8.1-m.main.
>
> I feel this a bit too much of a burden for the purpose, maybe there's
> a better way of handling all these alternatives (in particular,
> there's a lot of duplication since the expected code for the secure*
> functions is the same for v8-M and v8.1-M).
FWIW, an alternative is to give multiple versions with the same prefix
and use { target ... } to select between them. E.g.:
/*
** foo: { target a }
** ...
*/
/*
** foo: { target { ! a } }
** ...
*/
Thanks,
Richard
Re: [testsuite][arm] Fix cmse-15.c expected output
On Tue, 7 Apr 2020 at 12:31, Andre Vieira (lists) wrote: > > On 06/04/2020 16:12, Christophe Lyon via Gcc-patches wrote: > > Hi, > > > > While checking Martin's fix for PR ipa/94445, he made me realize that > > the cmse-15.c testcase still fails at -Os because ICF means that we > > generate > > nonsecure2: > > b nonsecure0 > > > > which is OK, but does not match the currently expected > > nonsecure2: > > ... > > bl __gnu_cmse_nonsecure_call > > > > (see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html) > > > > The test has already different expectations for v8-M and v8.1-M. > > > > I've decided to try to use check-function-bodies to account for the > > different possibilities: > > - v8-M vs v8.1-M via two different prefixes > > - code generation variants (-0?) via multiple regexps > > > > I've tested that the test now passes with --target-board=-march=armv8-m.main > > and --target-board=-march=armv8.1-m.main. > > > > I feel this a bit too much of a burden for the purpose, maybe there's > > a better way of handling all these alternatives (in particular, > > there's a lot of duplication since the expected code for the secure* > > functions is the same for v8-M and v8.1-M). > > > > OK? > > > > Thanks, > > > > Christophe > Hi Christophe, > > This check-function-bodies functionality is pretty sweet, I assume the ( > A | B ) checks for either of them? Yes. > If so that looks like a good improvement. Ideally we'd also check the > clearing for the v8.1-M cases, but that wasn't there before either and > they would need again splitting for -mfloat-abi=soft+softfp and > -mfloat-abi=hard. > Not sure what you mean? The only nonsecure test with the (A|B) construct is: +*Clear nonsecure2: +*Clear ... +*Clear ( +*Clear blxns r[0-3] +*Clear | +*Clear b nonsecure0 +*Clear ) So it does check the clearing (blxns), and 'b nonsecure0' is as valid as the result of the test for nonsecure0. > > So yeah this LGTM but you need approval from a port/global maintainer. > Thanks > Cheers, > Andre
Re: [testsuite][arm] Fix cmse-15.c expected output
On 06/04/2020 16:12, Christophe Lyon via Gcc-patches wrote: Hi, While checking Martin's fix for PR ipa/94445, he made me realize that the cmse-15.c testcase still fails at -Os because ICF means that we generate nonsecure2: b nonsecure0 which is OK, but does not match the currently expected nonsecure2: ... bl __gnu_cmse_nonsecure_call (see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html) The test has already different expectations for v8-M and v8.1-M. I've decided to try to use check-function-bodies to account for the different possibilities: - v8-M vs v8.1-M via two different prefixes - code generation variants (-0?) via multiple regexps I've tested that the test now passes with --target-board=-march=armv8-m.main and --target-board=-march=armv8.1-m.main. I feel this a bit too much of a burden for the purpose, maybe there's a better way of handling all these alternatives (in particular, there's a lot of duplication since the expected code for the secure* functions is the same for v8-M and v8.1-M). OK? Thanks, Christophe Hi Christophe, This check-function-bodies functionality is pretty sweet, I assume the ( A | B ) checks for either of them? If so that looks like a good improvement. Ideally we'd also check the clearing for the v8.1-M cases, but that wasn't there before either and they would need again splitting for -mfloat-abi=soft+softfp and -mfloat-abi=hard. So yeah this LGTM but you need approval from a port/global maintainer. Cheers, Andre
[testsuite][arm] Fix cmse-15.c expected output
Hi,
While checking Martin's fix for PR ipa/94445, he made me realize that
the cmse-15.c testcase still fails at -Os because ICF means that we
generate
nonsecure2:
b nonsecure0
which is OK, but does not match the currently expected
nonsecure2:
...
bl __gnu_cmse_nonsecure_call
(see https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543190.html)
The test has already different expectations for v8-M and v8.1-M.
I've decided to try to use check-function-bodies to account for the
different possibilities:
- v8-M vs v8.1-M via two different prefixes
- code generation variants (-0?) via multiple regexps
I've tested that the test now passes with --target-board=-march=armv8-m.main
and --target-board=-march=armv8.1-m.main.
I feel this a bit too much of a burden for the purpose, maybe there's
a better way of handling all these alternatives (in particular,
there's a lot of duplication since the expected code for the secure*
functions is the same for v8-M and v8.1-M).
OK?
Thanks,
Christophe
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
index 0e37b50..603c456 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
@@ -1,5 +1,9 @@
/* { dg-do compile } */
/* { dg-options "-mcmse" } */
+/* ARMv8-M expectation. */
+/* { dg-final { check-function-bodies "*Noclear" "" "" { target { !
arm_cmse_clear_ok } } } } */
+/* ARMv8.1-M expectation. */
+/* { dg-final { check-function-bodies "*Clear" "" "" { target
arm_cmse_clear_ok } } } */
int __attribute__ ((cmse_nonsecure_call)) (*ns_foo) (void);
int (*s_bar) (void);
@@ -11,67 +15,242 @@ typedef int s_bar_t (void);
typedef int __attribute__ ((cmse_nonsecure_call)) (* ns_foo_ptr) (void);
typedef int (*s_bar_ptr) (void);
+/*
+*Clear nonsecure0:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure0:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure0 (ns_foo_t * ns_foo_p)
{
return ns_foo_p ();
}
+/*
+*Clear nonsecure1:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure1:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure1 (ns_foo_t ** ns_foo_p)
{
return (*ns_foo_p) ();
}
+/*
+*Clear nonsecure2:
+*Clear ...
+*Clear (
+*Clear blxns r[0-3]
+*Clear |
+*Clear b nonsecure0
+*Clear )
+*Clear ...
+*Noclear nonsecure2:
+*Noclear ...
+*Noclear (
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear |
+*Noclear b nonsecure0
+*Noclear )
+*Noclear ...
+*/
int nonsecure2 (ns_foo_ptr ns_foo_p)
{
return ns_foo_p ();
}
+
+/*
+*Clear nonsecure3:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure3:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure3 (ns_foo_ptr * ns_foo_p)
{
return (*ns_foo_p) ();
}
+/*
+*Clear secure0:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear )
+*Clear ...
+*Noclear secure0:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear )
+*Noclear ...
+*/
int secure0 (s_bar_t * s_bar_p)
{
return s_bar_p ();
}
+/*
+*Clear secure1:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear )
+*Clear ...
+*Noclear secure1:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear )
+*Noclear ...
+*/
int secure1 (s_bar_t ** s_bar_p)
{
return (*s_bar_p) ();
}
+/*
+*Clear secure2:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear |
+*Clear b secure0
+*Clear )
+*Clear ...
+*Noclear secure2:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear |
+*Noclear b secure0
+*Noclear )
+*Noclear ...
+*/
int secure2 (s_bar_ptr s_bar_p)
{
return s_bar_p ();
}
+/*
+*Clear secure3:
+*Clear ...
+*Clear (
+*Clear bx r[0-3]
+*Clear |
+*Clear blx r[0-3]
+*Clear )
+*Clear ...
+*Noclear secure3:
+*Noclear ...
+*Noclear (
+*Noclear bx r[0-3]
+*Noclear |
+*Noclear blx r[0-3]
+*Noclear )
+*Noclear ...
+*/
int secure3 (s_bar_ptr * s_bar_p)
{
return (*s_bar_p) ();
}
+/*
+*Clear nonsecure4:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure4:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure4 (void)
{
return ns_foo ();
}
+/*
+*Clear nonsecure5:
+*Clear ...
+*Clear blxns r[0-3]
+*Clear ...
+*Noclear nonsecure5:
+*Noclear ...
+*Noclear bl __gnu_cmse_nonsecure_call
+*Noclear ...
+*/
int nonsecure5 (void)
{
return (*ns_foo2) ();
}
+/*
+*Clear
