diff mbox series

[v2,3/7] aarch64: Add eh_return compile tests

Message ID 60a89113beb96fc0183c8ebc2a0dc8d6feb91478.1699025214.git.szabolcs.nagy@arm.com
State New
Headers show
Series aarch64 GCS preliminary patches | expand

Commit Message

Szabolcs Nagy Nov. 3, 2023, 3:36 p.m. UTC 
gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/eh_return-2.c: New test.
	* gcc.target/aarch64/eh_return-3.c: New test.

---
v2: check-function-bodies in eh_return-3.c
(this is not very robust, but easier to read)
---
 .../gcc.target/aarch64/eh_return-2.c          |  9 ++++++
 .../gcc.target/aarch64/eh_return-3.c          | 30 +++++++++++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-3.c

Comments

Richard Sandiford Nov. 26, 2023, 2:37 p.m. UTC | #1 
Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/eh_return-2.c: New test.
> 	* gcc.target/aarch64/eh_return-3.c: New test.
>
> ---
> v2: check-function-bodies in eh_return-3.c
> (this is not very robust, but easier to read)
> ---
>  .../gcc.target/aarch64/eh_return-2.c          |  9 ++++++
>  .../gcc.target/aarch64/eh_return-3.c          | 30 +++++++++++++++++++
>  2 files changed, 39 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-2.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-3.c
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-2.c b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
> new file mode 100644
> index 00000000000..4a9d124e891
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-final { scan-assembler "add\tsp, sp, x5" } } */
> +/* { dg-final { scan-assembler "br\tx6" } } */
> +
> +void
> +foo (unsigned long off, void *handler)
> +{
> +  __builtin_eh_return (off, handler);
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-3.c b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
> new file mode 100644
> index 00000000000..bfbe92af427
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mbranch-protection=pac-ret+leaf" } */

Probably best to add -fno-schedule-insns -fno-schedule-insns2, so that the
instructions in the check-function-bodies are in a more predictable order.

> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +**foo:
> +**	hint	25 // paciasp
> +**	stp	x0, x1, .*
> +**	stp	x2, x3, .*
> +**	cbz	w2, .*
> +**	mov	x4, 0
> +**	ldp	x2, x3, .*
> +**	ldp	x0, x1, .*
> +**	cbz	x4, .*
> +**	add	sp, sp, x5
> +**	br	x6
> +**	hint	29 // autiasp
> +**	ret
> +**	mov	x5, x0
> +**	mov	x6, x1
> +**	mov	x4, 1
> +**	b	.*
> +*/

What's the significance of x3 here?  It looks from the function definition
like it should be undefined.  And what are the stps and ldps doing?

If those aren't an important part of the test, it might be better
to stub them out with "...", e.g.:

/*
**foo:
**	hint	25 // paciasp
**	...
**	cbz	w2, .*
**	mov	x4, 0
**	...
**	cbz	x4, .*
**	add	sp, sp, x5
**	br	x6
**	hint	29 // autiasp
**	ret
**	mov	x5, x0
**	mov	x6, x1
**	mov	x4, 1
**	b	.*
*/

LGTM otherwise.

Thanks,
Richard

> +void
> +foo (unsigned long off, void *handler, int c)
> +{
> +  if (c)
> +    return;
> +  __builtin_eh_return (off, handler);
> +}
Szabolcs Nagy Nov. 27, 2023, 10:04 a.m. UTC | #2 
The 11/26/2023 14:37, Richard Sandiford wrote:
> Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> > +++ b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
> > @@ -0,0 +1,30 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -mbranch-protection=pac-ret+leaf" } */
> 
> Probably best to add -fno-schedule-insns -fno-schedule-insns2, so that the
> instructions in the check-function-bodies are in a more predictable order.
> 
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +
> > +/*
> > +**foo:
> > +**	hint	25 // paciasp
> > +**	stp	x0, x1, .*
> > +**	stp	x2, x3, .*
> > +**	cbz	w2, .*
> > +**	mov	x4, 0
> > +**	ldp	x2, x3, .*
> > +**	ldp	x0, x1, .*
> > +**	cbz	x4, .*
> > +**	add	sp, sp, x5
> > +**	br	x6
> > +**	hint	29 // autiasp
> > +**	ret
> > +**	mov	x5, x0
> > +**	mov	x6, x1
> > +**	mov	x4, 1
> > +**	b	.*
> > +*/
> 
> What's the significance of x3 here?  It looks from the function definition
> like it should be undefined.  And what are the stps and ldps doing?

x0,..,x3 are preserved registers for eh (EH_RETURN_DATA_REGNO).

they are saved in the prologue and restored in the epilogue so
they can pass arguments to eh, which i think is relevant to an
eh_return test, although if the compiler knows they are not
clobbered then it could eliminate the save/restore.

> If those aren't an important part of the test, it might be better
> to stub them out with "...", e.g.:

i can do that.

> /*
> **foo:
> **	hint	25 // paciasp
> **	...
> **	cbz	w2, .*
> **	mov	x4, 0
> **	...
> **	cbz	x4, .*
> **	add	sp, sp, x5
> **	br	x6
> **	hint	29 // autiasp
> **	ret
> **	mov	x5, x0
> **	mov	x6, x1
> **	mov	x4, 1
> **	b	.*
> */
> 
> LGTM otherwise.

thanks.
Szabolcs Nagy Nov. 27, 2023, 3:57 p.m. UTC | #3 
The 11/26/2023 14:37, Richard Sandiford wrote:
> Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> > +++ b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
> > @@ -0,0 +1,30 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -mbranch-protection=pac-ret+leaf" } */
> 
> Probably best to add -fno-schedule-insns -fno-schedule-insns2, so that the
> instructions in the check-function-bodies are in a more predictable order.
> 
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +
> > +/*
> > +**foo:
> > +**	hint	25 // paciasp
> > +**	stp	x0, x1, .*
> > +**	stp	x2, x3, .*
> > +**	cbz	w2, .*
> > +**	mov	x4, 0
> > +**	ldp	x2, x3, .*
> > +**	ldp	x0, x1, .*
> > +**	cbz	x4, .*
> > +**	add	sp, sp, x5
> > +**	br	x6
> > +**	hint	29 // autiasp
> > +**	ret
> > +**	mov	x5, x0
> > +**	mov	x6, x1
> > +**	mov	x4, 1
> > +**	b	.*
> > +*/
> 
> What's the significance of x3 here?  It looks from the function definition
> like it should be undefined.  And what are the stps and ldps doing?
> 
> If those aren't an important part of the test, it might be better
> to stub them out with "...", e.g.:
> 
> /*
> **foo:
> **	hint	25 // paciasp
> **	...
> **	cbz	w2, .*
> **	mov	x4, 0
> **	...
> **	cbz	x4, .*
> **	add	sp, sp, x5
> **	br	x6
> **	hint	29 // autiasp
> **	ret
> **	mov	x5, x0
> **	mov	x6, x1
> **	mov	x4, 1
> **	b	.*
> */
> 
> LGTM otherwise.

committed as

From cad7e1e3e0dea1922f89290bbbc27b4c44f53bf5 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Fri, 2 Jun 2023 14:17:02 +0100
Subject: [PATCH] aarch64: Add eh_return compile tests

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/eh_return-2.c: New test.
	* gcc.target/aarch64/eh_return-3.c: New test.
---
 .../gcc.target/aarch64/eh_return-2.c          |  9 ++++++
 .../gcc.target/aarch64/eh_return-3.c          | 28 +++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-3.c

diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-2.c b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
new file mode 100644
index 00000000000..4a9d124e891
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-final { scan-assembler "add\tsp, sp, x5" } } */
+/* { dg-final { scan-assembler "br\tx6" } } */
+
+void
+foo (unsigned long off, void *handler)
+{
+  __builtin_eh_return (off, handler);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-3.c b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
new file mode 100644
index 00000000000..a17baa86501
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mbranch-protection=pac-ret+leaf -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+/*
+**foo:
+**	hint	25 // paciasp
+**	...
+**	cbz	w2, .*
+**	mov	x4, 0
+**	...
+**	cbz	x4, .*
+**	add	sp, sp, x5
+**	br	x6
+**	hint	29 // autiasp
+**	ret
+**	mov	x5, x0
+**	mov	x4, 1
+**	mov	x6, x1
+**	b	.*
+*/
+void
+foo (unsigned long off, void *handler, int c)
+{
+  if (c)
+    return;
+  __builtin_eh_return (off, handler);
+}
Andrew Pinski Dec. 2, 2023, 7:23 p.m. UTC | #4 
On Fri, Nov 3, 2023 at 8:37 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/aarch64/eh_return-2.c: New test.
>         * gcc.target/aarch64/eh_return-3.c: New test.


gcc.target/aarch64/eh_return-3.c fails when running the testsuite with
`-march=armv9-a+sve` . I think it is a good idea to try to keep the
testsuite clean when running with different -march=/-mcpu= options
even. I know there are many failures due to -march=/-mcpu option right
now but this is a new testcase and all.

Thanks,
Andrew

>
> ---
> v2: check-function-bodies in eh_return-3.c
> (this is not very robust, but easier to read)
> ---
>  .../gcc.target/aarch64/eh_return-2.c          |  9 ++++++
>  .../gcc.target/aarch64/eh_return-3.c          | 30 +++++++++++++++++++
>  2 files changed, 39 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-2.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/eh_return-3.c
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-2.c b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
> new file mode 100644
> index 00000000000..4a9d124e891
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-final { scan-assembler "add\tsp, sp, x5" } } */
> +/* { dg-final { scan-assembler "br\tx6" } } */
> +
> +void
> +foo (unsigned long off, void *handler)
> +{
> +  __builtin_eh_return (off, handler);
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-3.c b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
> new file mode 100644
> index 00000000000..bfbe92af427
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mbranch-protection=pac-ret+leaf" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +/*
> +**foo:
> +**     hint    25 // paciasp
> +**     stp     x0, x1, .*
> +**     stp     x2, x3, .*
> +**     cbz     w2, .*
> +**     mov     x4, 0
> +**     ldp     x2, x3, .*
> +**     ldp     x0, x1, .*
> +**     cbz     x4, .*
> +**     add     sp, sp, x5
> +**     br      x6
> +**     hint    29 // autiasp
> +**     ret
> +**     mov     x5, x0
> +**     mov     x6, x1
> +**     mov     x4, 1
> +**     b       .*
> +*/
> +void
> +foo (unsigned long off, void *handler, int c)
> +{
> +  if (c)
> +    return;
> +  __builtin_eh_return (off, handler);
> +}
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-2.c b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
new file mode 100644
index 00000000000..4a9d124e891
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/eh_return-2.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-final { scan-assembler "add\tsp, sp, x5" } } */
+/* { dg-final { scan-assembler "br\tx6" } } */
+
+void
+foo (unsigned long off, void *handler)
+{
+  __builtin_eh_return (off, handler);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return-3.c b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
new file mode 100644
index 00000000000..bfbe92af427
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/eh_return-3.c
@@ -0,0 +1,30 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -mbranch-protection=pac-ret+leaf" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+/*
+**foo:
+**	hint	25 // paciasp
+**	stp	x0, x1, .*
+**	stp	x2, x3, .*
+**	cbz	w2, .*
+**	mov	x4, 0
+**	ldp	x2, x3, .*
+**	ldp	x0, x1, .*
+**	cbz	x4, .*
+**	add	sp, sp, x5
+**	br	x6
+**	hint	29 // autiasp
+**	ret
+**	mov	x5, x0
+**	mov	x6, x1
+**	mov	x4, 1
+**	b	.*
+*/
+void
+foo (unsigned long off, void *handler, int c)
+{
+  if (c)
+    return;
+  __builtin_eh_return (off, handler);
+}