public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests
@ 2024-04-16  3:48 Alexandre Oliva
  2024-04-16  8:56 ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Oliva @ 2024-04-16  3:48 UTC (permalink / raw)
  To: gcc-patches
  Cc: Rainer Orth, Mike Stump, Nick Clifton, Richard Earnshaw,
	Ramana Radhakrishnan


arm pac and bti tests that use -march=armv8.1-m.main get an implicit
-mthumb, that is incompatible with vxworks kernel mode.  Declaring the
requirement for a 8.1-m.main-compatible toolchain is enough to avoid
those fails, because the toolchain feature test fails in kernel mode.

Regstrapped on x86_64-linux-gnu.  Also tested with gcc-13 on arm-,
aarch64-, x86- and x86_64-vxworks7r2.  Ok to install?


for  gcc/testsuite/ChangeLog

	* g++.target/arm/pac-1.C: Require arm_arch_v8_1m_main.
	* gcc.target/arm/acle/pacbti-m-predef-11.c: Likewise.
	* gcc.target/arm/acle/pacbti-m-predef-12.c: Likewise.
	* gcc.target/arm/acle/pacbti-m-predef-7.c: Likewise.
	* gcc.target/arm/bti-1.c: Likewise.
	* gcc.target/arm/bti-2.c: Likewise.
---
 gcc/testsuite/g++.target/arm/pac-1.C               |    1 +
 .../gcc.target/arm/acle/pacbti-m-predef-11.c       |    1 +
 .../gcc.target/arm/acle/pacbti-m-predef-12.c       |    1 +
 .../gcc.target/arm/acle/pacbti-m-predef-7.c        |    1 +
 gcc/testsuite/gcc.target/arm/bti-1.c               |    1 +
 gcc/testsuite/gcc.target/arm/bti-2.c               |    1 +
 6 files changed, 6 insertions(+)

diff --git a/gcc/testsuite/g++.target/arm/pac-1.C b/gcc/testsuite/g++.target/arm/pac-1.C
index f671a27b048c6..f48ad6cc5cb65 100644
--- a/gcc/testsuite/g++.target/arm/pac-1.C
+++ b/gcc/testsuite/g++.target/arm/pac-1.C
@@ -2,6 +2,7 @@
 /* { dg-do compile } */
 /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
 /* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
 
 __attribute__((noinline)) void
 fn1 (int a, int b, int c)
diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
index 6a5ae92c567f3..dba4f491cfea7 100644
--- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
 /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" "-mfloat-abi=*" } } */
 /* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
 
 #if (__ARM_FEATURE_BTI != 1)
 #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be defined to 1."
diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
index db40b17c3b030..308a41eb4ba4c 100644
--- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
 /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
 /* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
 
 #if defined (__ARM_FEATURE_BTI)
 #error "Feature test macro __ARM_FEATURE_BTI should not be defined."
diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
index 1b25907635e24..10836a84bde56 100644
--- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
 /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
 /* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
 
 #if defined (__ARM_FEATURE_BTI_DEFAULT)
 #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be undefined."
diff --git a/gcc/testsuite/gcc.target/arm/bti-1.c b/gcc/testsuite/gcc.target/arm/bti-1.c
index 79dd8010d2dab..34655387b55f5 100644
--- a/gcc/testsuite/gcc.target/arm/bti-1.c
+++ b/gcc/testsuite/gcc.target/arm/bti-1.c
@@ -2,6 +2,7 @@
 /* { dg-do compile } */
 /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
 /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
 
 int
 main (void)
diff --git a/gcc/testsuite/gcc.target/arm/bti-2.c b/gcc/testsuite/gcc.target/arm/bti-2.c
index 33910563849a4..3284aba3020cc 100644
--- a/gcc/testsuite/gcc.target/arm/bti-2.c
+++ b/gcc/testsuite/gcc.target/arm/bti-2.c
@@ -3,6 +3,7 @@
 /* { dg-options "-Os" } */
 /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
 /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
 
 extern int f1 (void);
 extern int f2 (void);

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests
  2024-04-16  3:48 [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests Alexandre Oliva
@ 2024-04-16  8:56 ` Richard Earnshaw (lists)
  2024-04-19 12:45   ` Alexandre Oliva
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Earnshaw (lists) @ 2024-04-16  8:56 UTC (permalink / raw)
  To: Alexandre Oliva, gcc-patches
  Cc: Rainer Orth, Mike Stump, Nick Clifton, Ramana Radhakrishnan

On 16/04/2024 04:48, Alexandre Oliva wrote:
> 
> arm pac and bti tests that use -march=armv8.1-m.main get an implicit
> -mthumb, that is incompatible with vxworks kernel mode.  Declaring the
> requirement for a 8.1-m.main-compatible toolchain is enough to avoid
> those fails, because the toolchain feature test fails in kernel mode.
> 
> Regstrapped on x86_64-linux-gnu.  Also tested with gcc-13 on arm-,
> aarch64-, x86- and x86_64-vxworks7r2.  Ok to install?
> 
> 
> for  gcc/testsuite/ChangeLog
> 
> 	* g++.target/arm/pac-1.C: Require arm_arch_v8_1m_main.
> 	* gcc.target/arm/acle/pacbti-m-predef-11.c: Likewise.
> 	* gcc.target/arm/acle/pacbti-m-predef-12.c: Likewise.
> 	* gcc.target/arm/acle/pacbti-m-predef-7.c: Likewise.
> 	* gcc.target/arm/bti-1.c: Likewise.
> 	* gcc.target/arm/bti-2.c: Likewise.
> ---
>  gcc/testsuite/g++.target/arm/pac-1.C               |    1 +
>  .../gcc.target/arm/acle/pacbti-m-predef-11.c       |    1 +
>  .../gcc.target/arm/acle/pacbti-m-predef-12.c       |    1 +
>  .../gcc.target/arm/acle/pacbti-m-predef-7.c        |    1 +
>  gcc/testsuite/gcc.target/arm/bti-1.c               |    1 +
>  gcc/testsuite/gcc.target/arm/bti-2.c               |    1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/gcc/testsuite/g++.target/arm/pac-1.C b/gcc/testsuite/g++.target/arm/pac-1.C
> index f671a27b048c6..f48ad6cc5cb65 100644
> --- a/gcc/testsuite/g++.target/arm/pac-1.C
> +++ b/gcc/testsuite/g++.target/arm/pac-1.C
> @@ -2,6 +2,7 @@
>  /* { dg-do compile } */
>  /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
>  /* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */

The require-effective-target flags test whether a specific set of flags will make the compilation work, so they need to be used in conjunction with the corresponding dg-add-options flags that then apply those options.  It isn't safe to just add a different architecture flag instead.  So if you're going to use this effective target, you should use it along with "dg-add-options arm_arch_v8_1m_main" (ie the effective-target name minus the trailing '_ok'), and then replace dg-options with dg-additional-options adding the remaining flags.  You can then remove the dg-skip-if as well because that's what the require-effective-target flag is doing.  So something like

dg-do compile
dg-require-effective-target arm_arch_v8_1m_main_ok
dg-add-options arm_arch_v8_1m_main
dg-additional-options "-mbranch-protection=pac-ret -g -O0"

But this test is also adding pacbti to the architecture flags, so it would probably be better to use v8_1m_main_pacbti_ok as the effective target.  It's not identical to the options above, but it's probably sufficient for this test.  Each test below will need checking for the exact flags that are needed for the test in question.


>  
>  __attribute__((noinline)) void
>  fn1 (int a, int b, int c)
> diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
> index 6a5ae92c567f3..dba4f491cfea7 100644
> --- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
> +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
> @@ -1,6 +1,7 @@
>  /* { dg-do compile } */
>  /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" "-mfloat-abi=*" } } */
>  /* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
>  
>  #if (__ARM_FEATURE_BTI != 1)
>  #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be defined to 1."
> diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
> index db40b17c3b030..308a41eb4ba4c 100644
> --- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
> +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
> @@ -1,6 +1,7 @@
>  /* { dg-do compile } */
>  /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
>  /* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
>  
>  #if defined (__ARM_FEATURE_BTI)
>  #error "Feature test macro __ARM_FEATURE_BTI should not be defined."
> diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
> index 1b25907635e24..10836a84bde56 100644
> --- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
> +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
> @@ -1,6 +1,7 @@
>  /* { dg-do compile } */
>  /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
>  /* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
>  
>  #if defined (__ARM_FEATURE_BTI_DEFAULT)
>  #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be undefined."
> diff --git a/gcc/testsuite/gcc.target/arm/bti-1.c b/gcc/testsuite/gcc.target/arm/bti-1.c
> index 79dd8010d2dab..34655387b55f5 100644
> --- a/gcc/testsuite/gcc.target/arm/bti-1.c
> +++ b/gcc/testsuite/gcc.target/arm/bti-1.c
> @@ -2,6 +2,7 @@
>  /* { dg-do compile } */
>  /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
>  /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
>  
>  int
>  main (void)
> diff --git a/gcc/testsuite/gcc.target/arm/bti-2.c b/gcc/testsuite/gcc.target/arm/bti-2.c
> index 33910563849a4..3284aba3020cc 100644
> --- a/gcc/testsuite/gcc.target/arm/bti-2.c
> +++ b/gcc/testsuite/gcc.target/arm/bti-2.c
> @@ -3,6 +3,7 @@
>  /* { dg-options "-Os" } */
>  /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
>  /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
>  
>  extern int f1 (void);
>  extern int f2 (void);
> 

R.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests
  2024-04-16  8:56 ` Richard Earnshaw (lists)
@ 2024-04-19 12:45   ` Alexandre Oliva
  2024-04-19 13:13     ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Oliva @ 2024-04-19 12:45 UTC (permalink / raw)
  To: Richard Earnshaw (lists)
  Cc: gcc-patches, Rainer Orth, Mike Stump, Nick Clifton, Ramana Radhakrishnan

On Apr 16, 2024, "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> wrote:

> The require-effective-target flags test whether a specific set of
> flags will make the compilation work, so they need to be used in
> conjunction with the corresponding dg-add-options flags that then
> apply those options.

*nod*, that's the theory.  Problem is the architectures suported by
[add_options_for_]arm_arch_*[_ok] do not match exactly those expected by
the tests, and I can't quite tell whether the subtle changes they would
introduce would change what they intend to test, or even whether the
differences are irrelevant, or would be sensible to add as variants to
the dg machinery.  I think it would take someone more familiar than I am
with all of the ARM variants to do this correctly.  I don't even know
how these changes would need to be tested to be sure they remain
correct.

Would you be willing to take it from here, or would you accept the patch
as an incremental yet imperfect improvement, or would you prefer to
guide me in making it correct, and in verifying it (there are questions
below)?  I don't have a lot of cycles to put into this (we've already
worked around the testsuite bugs we ran into), but it would be desirable
to get a fix into GCC as well, if we can converge on one without
unreasonably burdening anyone.


	v8_1m_main "-march=armv8.1-m.main+fp -mthumb" __ARM_ARCH_8M_MAIN__
	v8_1m_main_pacbti "-march=armv8.1-m.main+pacbti+fp -mthumb"
		"__ARM_ARCH_8M_MAIN__ && __ARM_FEATURE_BTI && __ARM_FEATURE_PAUTH

Why do these have +fp in -march but not in the v8_1m* arch name?


gcc/testsuite/g++.target/arm/pac-1.C:
/* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */

v8_1m_main_pacbti plus +mve minus +fp.
Do we need a dg arch for that?


gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c:
/* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */
gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c:
/* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */

v8_1m_main_pacbti minus -mthumb.
AFAICT the -mthumb is redundant.


gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c:
/* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */

v8_1m_main minus -mthumb.
AFAICT the -mthumb is redundant.


gcc/testsuite/gcc.target/arm/bti-1.c:
/* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
gcc/testsuite/gcc.target/arm/bti-2.c:
/* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */

v8_1m_main minus +fp.

Can these be bumped to +fp, or do we need an extra dg arch?

Are these missing +pacbti?


Thanks,

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests
  2024-04-19 12:45   ` Alexandre Oliva
@ 2024-04-19 13:13     ` Richard Earnshaw (lists)
  2024-04-19 16:37       ` [PATCH v2] [testsuite] [arm] add effective target and options " Alexandre Oliva
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Earnshaw (lists) @ 2024-04-19 13:13 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: gcc-patches, Rainer Orth, Mike Stump, Nick Clifton, Ramana Radhakrishnan

On 19/04/2024 13:45, Alexandre Oliva wrote:
> On Apr 16, 2024, "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> wrote:
> 
>> The require-effective-target flags test whether a specific set of
>> flags will make the compilation work, so they need to be used in
>> conjunction with the corresponding dg-add-options flags that then
>> apply those options.
> 
> *nod*, that's the theory.  Problem is the architectures suported by
> [add_options_for_]arm_arch_*[_ok] do not match exactly those expected by
> the tests, and I can't quite tell whether the subtle changes they would
> introduce would change what they intend to test, or even whether the
> differences are irrelevant, or would be sensible to add as variants to
> the dg machinery.  I think it would take someone more familiar than I am
> with all of the ARM variants to do this correctly.  I don't even know
> how these changes would need to be tested to be sure they remain
> correct.

It's ok to add additional variations to the table of variants in target-supports.exp, but we should avoid writing new specific run-time functions unless we really want an executable test.

I started doing some cleanup of the Arm tests infrastructure during phase 3, but stopped during phase 4 as I wanted to minimise the changes being made now.  I plan to go back and work on it some more once stage 1 re-opens.

> 
> Would you be willing to take it from here, or would you accept the patch
> as an incremental yet imperfect improvement, or would you prefer to
> guide me in making it correct, and in verifying it (there are questions
> below)?  I don't have a lot of cycles to put into this (we've already
> worked around the testsuite bugs we ran into), but it would be desirable
> to get a fix into GCC as well, if we can converge on one without
> unreasonably burdening anyone.
> 
> 
> 	v8_1m_main "-march=armv8.1-m.main+fp -mthumb" __ARM_ARCH_8M_MAIN__
> 	v8_1m_main_pacbti "-march=armv8.1-m.main+pacbti+fp -mthumb"
> 		"__ARM_ARCH_8M_MAIN__ && __ARM_FEATURE_BTI && __ARM_FEATURE_PAUTH
> 
> Why do these have +fp in -march but not in the v8_1m* arch name?

It's ... complicated :)

The +fp is there because, with the move to having -mfpu=auto as the default, we need to avoid problems when the compiler has been configured with --with-float=hard, which requires the extension register set (fp or vector support) even if the test code itself doesn't care.  The best way to handle this in most cases is to give the architecture strings a default FPU specification (ie +fp). 

> 
> 
> gcc/testsuite/g++.target/arm/pac-1.C:
> /* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */
> 
> v8_1m_main_pacbti plus +mve minus +fp.
> Do we need a dg arch for that?

I'd be inclined to drop +mve from this one; there's nothing I can see in the test that would generate mve instructions, so I think it's irrelevant.  We can use the existing v8_1m_main_pacbti operations.

> 
> 
> gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c:
> /* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */
> gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c:
> /* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */
> 
> v8_1m_main_pacbti minus -mthumb.
> AFAICT the -mthumb is redundant.

Nearly, but not quite.  Although the gcc driver knows that m-profile architectures require thumb, that's not enough to override an explicit -marm from a testsuite configuration run, so if your site.exp file adds -marm in a test configuration we need to override that or the test will fail.  But the table based list of options will do that for you.

> 
> 
> gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c:
> /* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */
> 
> v8_1m_main minus -mthumb.
> AFAICT the -mthumb is redundant.

As above

> 
> 
> gcc/testsuite/gcc.target/arm/bti-1.c:
> /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
> gcc/testsuite/gcc.target/arm/bti-2.c:
> /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
> 
> v8_1m_main minus +fp.> 
> Can these be bumped to +fp, or do we need an extra dg arch?
> 
> Are these missing +pacbti?

The tests themselves do not require fp, but if we use the effective-target rules (arm_arch_v8_1m_main), we can remove the -march, -mthumb and -mfloat-abi flags from these tests.

These tests for BTI should NOT have +pacbti: they're testing that the compiler generates the right nop-based implementation that is backwards compatible with CPUs that do not have this feature.[1]

R.

> 
> 
> Thanks,
> 

[1] The PAC and BTI features define the behaviour of some architectural NOP instructions: on older CPUs these instructions have no effect (they are NOPs), but on newer CPUs these NOPs take on a new behaviour that implements the feature (PAC or BTI).

R.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] [testsuite] [arm] add effective target and options for pacbti tests
  2024-04-19 13:13     ` Richard Earnshaw (lists)
@ 2024-04-19 16:37       ` Alexandre Oliva
  0 siblings, 0 replies; 5+ messages in thread
From: Alexandre Oliva @ 2024-04-19 16:37 UTC (permalink / raw)
  To: Richard Earnshaw (lists)
  Cc: gcc-patches, Rainer Orth, Mike Stump, Nick Clifton, Ramana Radhakrishnan

Hello, Richard,

Thanks, your response was very informative.

Here's a revised patch.

arm pac and bti tests that use -march=armv8.1-m.main get an implicit
-mthumb, that is incompatible with vxworks kernel mode.  Declaring the
requirement for a 8.1-m.main-compatible toolchain is enough to avoid
those fails, because the toolchain feature test fails in kernel mode,
but taking the -march options from the standardized arch tests, after
testing for support for the corresponding effective target, makes it
generally safer, and enables us to drop skip directives and extraneous
option variants.

Tested all 6 modified testcases with an x86_64-linux-gnu-x-arm-eabi
uberbaum build.  Ok to install?


for  gcc/testsuite/ChangeLog

	* gcc.target/arm/bti-1.c: Require arch, use its opts, drop skip.
	* gcc.target/arm/bti-2.c: Likewise.
	* gcc.target/arm/acle/pacbti-m-predef-11.c: Likewise.
	* gcc.target/arm/acle/pacbti-m-predef-12.c: Likewise.
	* gcc.target/arm/acle/pacbti-m-predef-7.c: Likewise.
	* g++.target/arm/pac-1.C: Likewise.  Drop +mve.
---
 gcc/testsuite/g++.target/arm/pac-1.C               |    5 +++--
 .../gcc.target/arm/acle/pacbti-m-predef-11.c       |    4 ++--
 .../gcc.target/arm/acle/pacbti-m-predef-12.c       |    5 +++--
 .../gcc.target/arm/acle/pacbti-m-predef-7.c        |    5 +++--
 gcc/testsuite/gcc.target/arm/bti-1.c               |    5 +++--
 gcc/testsuite/gcc.target/arm/bti-2.c               |    5 +++--
 6 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/gcc/testsuite/g++.target/arm/pac-1.C b/gcc/testsuite/g++.target/arm/pac-1.C
index f671a27b048c6..ac15ae18197ca 100644
--- a/gcc/testsuite/g++.target/arm/pac-1.C
+++ b/gcc/testsuite/g++.target/arm/pac-1.C
@@ -1,7 +1,8 @@
 /* Check that GCC does .save and .cfi_offset directives with RA_AUTH_CODE pseudo hard-register.  */
 /* { dg-do compile } */
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
-/* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_pacbti_ok } */
+/* { dg-add-options arm_arch_v8_1m_main_pacbti } */
+/* { dg-additional-options "-mbranch-protection=pac-ret -mfloat-abi=hard -g -O0" } */
 
 __attribute__((noinline)) void
 fn1 (int a, int b, int c)
diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
index 6a5ae92c567f3..c9c40f44027d4 100644
--- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" "-mfloat-abi=*" } } */
-/* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_pacbti_ok } */
+/* { dg-add-options arm_arch_v8_1m_main_pacbti } */
 
 #if (__ARM_FEATURE_BTI != 1)
 #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be defined to 1."
diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
index db40b17c3b030..c26051347a2cc 100644
--- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
-/* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-add-options arm_arch_v8_1m_main } */
+/* { dg-additional-options "-mfloat-abi=softfp" } */
 
 #if defined (__ARM_FEATURE_BTI)
 #error "Feature test macro __ARM_FEATURE_BTI should not be defined."
diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
index 1b25907635e24..92f500c1449b3 100644
--- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c
@@ -1,6 +1,7 @@
 /* { dg-do compile } */
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
-/* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_pacbti_ok } */
+/* { dg-add-options arm_arch_v8_1m_main_pacbti } */
+/* { dg-additional-options "--save-temps -mfloat-abi=hard" } */
 
 #if defined (__ARM_FEATURE_BTI_DEFAULT)
 #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be undefined."
diff --git a/gcc/testsuite/gcc.target/arm/bti-1.c b/gcc/testsuite/gcc.target/arm/bti-1.c
index 79dd8010d2dab..a34bb0842b632 100644
--- a/gcc/testsuite/gcc.target/arm/bti-1.c
+++ b/gcc/testsuite/gcc.target/arm/bti-1.c
@@ -1,7 +1,8 @@
 /* Check that GCC does bti instruction.  */
 /* { dg-do compile } */
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
-/* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-add-options arm_arch_v8_1m_main } */
+/* { dg-additional-options "-mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
 
 int
 main (void)
diff --git a/gcc/testsuite/gcc.target/arm/bti-2.c b/gcc/testsuite/gcc.target/arm/bti-2.c
index 33910563849a4..e5bc4d5543a8d 100644
--- a/gcc/testsuite/gcc.target/arm/bti-2.c
+++ b/gcc/testsuite/gcc.target/arm/bti-2.c
@@ -1,8 +1,9 @@
 /* { dg-do compile } */
 /* -Os to create jump table.  */
 /* { dg-options "-Os" } */
-/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */
-/* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-add-options arm_arch_v8_1m_main } */
+/* { dg-additional-options "-mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */
 
 extern int f1 (void);
 extern int f2 (void);

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-04-19 16:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-16  3:48 [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests Alexandre Oliva
2024-04-16  8:56 ` Richard Earnshaw (lists)
2024-04-19 12:45   ` Alexandre Oliva
2024-04-19 13:13     ` Richard Earnshaw (lists)
2024-04-19 16:37       ` [PATCH v2] [testsuite] [arm] add effective target and options " Alexandre Oliva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).