diff mbox series

rs6000: Enable more sibcalls when TOC is not preserved

Message ID 20200819144016.149039-1-wschmidt@linux.ibm.com
State New
Headers show
Series rs6000: Enable more sibcalls when TOC is not preserved | expand

Commit Message

Bill Schmidt Aug. 19, 2020, 2:40 p.m. UTC
A function compiled with the PC-relative addressing model does not
require r2 to contain a TOC pointer, and does not guarantee that r2
will be preserved for its caller.  Such a function can make sibcalls
without restriction based on TOC preservation rules.  However, a
caller that does preserve r2 cannot make a sibcall to a callee that
does not.

2020-08-19  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-logue.c (rs6000_decl_ok_for_sibcall):
	Sibcalls are always legal when the caller doesn't preserve r2.

gcc/testsuite/
	* gcc.target/powerpc/pcrel-sibcall-1.c: Adjust.
---
 gcc/config/rs6000/rs6000-logue.c              | 30 +++++++++----------
 .../gcc.target/powerpc/pcrel-sibcall-1.c      | 19 ++++++++----
 2 files changed, 28 insertions(+), 21 deletions(-)

Comments

Li, Pan2 via Gcc-patches Aug. 19, 2020, 2:41 p.m. UTC | #1
I failed to mention that this has been bootstrapped and tested on 
powerpc64le-unknown-linux-gnu, with no regressions.  Is this ok for trunk?

Thanks,
Bill

On 8/19/20 9:40 AM, Bill Schmidt via Gcc-patches wrote:
> A function compiled with the PC-relative addressing model does not
> require r2 to contain a TOC pointer, and does not guarantee that r2
> will be preserved for its caller.  Such a function can make sibcalls
> without restriction based on TOC preservation rules.  However, a
> caller that does preserve r2 cannot make a sibcall to a callee that
> does not.
>
> 2020-08-19  Bill Schmidt  <wschmidt@linux.ibm.com>
>
> gcc/
> 	* config/rs6000/rs6000-logue.c (rs6000_decl_ok_for_sibcall):
> 	Sibcalls are always legal when the caller doesn't preserve r2.
>
> gcc/testsuite/
> 	* gcc.target/powerpc/pcrel-sibcall-1.c: Adjust.
> ---
>   gcc/config/rs6000/rs6000-logue.c              | 30 +++++++++----------
>   .../gcc.target/powerpc/pcrel-sibcall-1.c      | 19 ++++++++----
>   2 files changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c
> index 6aad1ff826a..5a2cb7fdf2c 100644
> --- a/gcc/config/rs6000/rs6000-logue.c
> +++ b/gcc/config/rs6000/rs6000-logue.c
> @@ -1080,28 +1080,28 @@ rs6000_decl_ok_for_sibcall (tree decl)
>
>     if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
>       {
> -      /* Under the AIX or ELFv2 ABIs we can't allow calls to non-local
> -	 functions, because the callee may have a different TOC pointer to
> -	 the caller and there's no way to ensure we restore the TOC when
> +      /* A function compiled using the PC-relative addressing model does not
> +	 use a TOC pointer; nor is it guaranteed to preserve the value of
> +	 r2 for its caller's TOC.  Such a function may make sibcalls to any
> +	 function, whether local or external, without restriction based on
> +	 TOC-save/restore rules.  */
> +      if (rs6000_pcrel_p (cfun))
> +	return true;
> +
> +      /* Otherwise, under the AIX or ELFv2 ABIs we can't allow sibcalls
> +	 to non-local functions, because the callee may not preserve the
> +	 TOC pointer, and there's no way to ensure we restore the TOC when
>   	 we return.  */
>         if (!decl || DECL_EXTERNAL (decl) || DECL_WEAK (decl)
>   	  || !(*targetm.binds_local_p) (decl))
>   	return false;
>
> -      /* Similarly, if the caller preserves the TOC pointer and the callee
> -	 doesn't (or vice versa), proper TOC setup or restoration will be
> -	 missed.  For example, suppose A, B, and C are in the same binary
> -	 and A -> B -> C.  A and B preserve the TOC pointer but C does not,
> -	 and B -> C is eligible as a sibcall.  A will call B through its
> -	 local entry point, so A will not restore its TOC itself.  B calls
> -	 C with a sibcall, so it will not restore the TOC.  C does not
> -	 preserve the TOC, so it may clobber r2 with impunity.  Returning
> -	 from C will result in a corrupted TOC for A.  */
> -      else if (rs6000_fndecl_pcrel_p (decl) != rs6000_pcrel_p (cfun))
> +      /* A local sibcall from a function that preserves the TOC pointer
> +	 to a function that does not is invalid for the same reason.  */
> +      if (rs6000_fndecl_pcrel_p (decl))
>   	return false;
>
> -      else
> -	return true;
> +      return true;
>       }
>
>     /*  With the secure-plt SYSV ABI we can't make non-local calls when
> diff --git a/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c b/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c
> index dfcf8183ccd..9197788f98f 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c
> @@ -3,10 +3,9 @@
>   /* { dg-require-effective-target powerpc_elfv2 } */
>   /* { dg-require-effective-target power10_ok } */
>
> -/* Test that potential sibcalls are not generated when the caller preserves the
> -   TOC and the callee doesn't, or vice versa.  At present, -mcpu=power10 does
> -   not enable pc-relative mode.  Enable it here explicitly until it is turned
> -   on by default.  */
> +/* Test that potential sibcalls are generated when the caller does not
> +   preserve the TOC, even for external calls; and that sibcalls are not
> +   generated when the caller preserves the TOC but the callee does not.  */
>
>   #pragma GCC target ("cpu=power10,pcrel")
>   int x (void) __attribute__((noinline));
> @@ -39,12 +38,20 @@ int xx (void)
>     return 1;
>   }
>
> +extern int yy (void);
> +
>   #pragma GCC target ("cpu=power10,pcrel")
> -int notoc_call (void)
> +int notoc_sibcall (void)
>   {
>     return xx ();
>   }
>
> +int extern_sibcall (void)
> +{
> +  return yy ();
> +}
> +
>   /* { dg-final { scan-assembler {\mb x@notoc\M} } } */
>   /* { dg-final { scan-assembler {\mbl y\M} } } */
> -/* { dg-final { scan-assembler {\mbl xx@notoc\M} } } */
> +/* { dg-final { scan-assembler {\mb xx@notoc\M} } } */
> +/* { dg-final { scan-assembler {\mb yy@notoc\M} } } */
Segher Boessenkool Aug. 19, 2020, 6:10 p.m. UTC | #2
Hi!

On Wed, Aug 19, 2020 at 09:40:16AM -0500, Bill Schmidt wrote:
> A function compiled with the PC-relative addressing model does not
> require r2 to contain a TOC pointer, and does not guarantee that r2
> will be preserved for its caller.  Such a function can make sibcalls
> without restriction based on TOC preservation rules.  However, a
> caller that does preserve r2 cannot make a sibcall to a callee that
> does not.

This looks fine.  _Is_ fine even, afaics :-)  Okay for trunk.

Thanks!


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c
index 6aad1ff826a..5a2cb7fdf2c 100644
--- a/gcc/config/rs6000/rs6000-logue.c
+++ b/gcc/config/rs6000/rs6000-logue.c
@@ -1080,28 +1080,28 @@  rs6000_decl_ok_for_sibcall (tree decl)
 
   if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
     {
-      /* Under the AIX or ELFv2 ABIs we can't allow calls to non-local
-	 functions, because the callee may have a different TOC pointer to
-	 the caller and there's no way to ensure we restore the TOC when
+      /* A function compiled using the PC-relative addressing model does not
+	 use a TOC pointer; nor is it guaranteed to preserve the value of
+	 r2 for its caller's TOC.  Such a function may make sibcalls to any
+	 function, whether local or external, without restriction based on
+	 TOC-save/restore rules.  */
+      if (rs6000_pcrel_p (cfun))
+	return true;
+
+      /* Otherwise, under the AIX or ELFv2 ABIs we can't allow sibcalls
+	 to non-local functions, because the callee may not preserve the
+	 TOC pointer, and there's no way to ensure we restore the TOC when
 	 we return.  */
       if (!decl || DECL_EXTERNAL (decl) || DECL_WEAK (decl)
 	  || !(*targetm.binds_local_p) (decl))
 	return false;
 
-      /* Similarly, if the caller preserves the TOC pointer and the callee
-	 doesn't (or vice versa), proper TOC setup or restoration will be
-	 missed.  For example, suppose A, B, and C are in the same binary
-	 and A -> B -> C.  A and B preserve the TOC pointer but C does not,
-	 and B -> C is eligible as a sibcall.  A will call B through its
-	 local entry point, so A will not restore its TOC itself.  B calls
-	 C with a sibcall, so it will not restore the TOC.  C does not
-	 preserve the TOC, so it may clobber r2 with impunity.  Returning
-	 from C will result in a corrupted TOC for A.  */
-      else if (rs6000_fndecl_pcrel_p (decl) != rs6000_pcrel_p (cfun))
+      /* A local sibcall from a function that preserves the TOC pointer
+	 to a function that does not is invalid for the same reason.  */
+      if (rs6000_fndecl_pcrel_p (decl))
 	return false;
 
-      else
-	return true;
+      return true;
     }
 
   /*  With the secure-plt SYSV ABI we can't make non-local calls when
diff --git a/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c b/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c
index dfcf8183ccd..9197788f98f 100644
--- a/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/pcrel-sibcall-1.c
@@ -3,10 +3,9 @@ 
 /* { dg-require-effective-target powerpc_elfv2 } */
 /* { dg-require-effective-target power10_ok } */
 
-/* Test that potential sibcalls are not generated when the caller preserves the
-   TOC and the callee doesn't, or vice versa.  At present, -mcpu=power10 does
-   not enable pc-relative mode.  Enable it here explicitly until it is turned
-   on by default.  */
+/* Test that potential sibcalls are generated when the caller does not
+   preserve the TOC, even for external calls; and that sibcalls are not
+   generated when the caller preserves the TOC but the callee does not.  */
 
 #pragma GCC target ("cpu=power10,pcrel")
 int x (void) __attribute__((noinline));
@@ -39,12 +38,20 @@  int xx (void)
   return 1;
 }
 
+extern int yy (void);
+
 #pragma GCC target ("cpu=power10,pcrel")
-int notoc_call (void)
+int notoc_sibcall (void)
 {
   return xx ();
 }
 
+int extern_sibcall (void)
+{
+  return yy ();
+}
+
 /* { dg-final { scan-assembler {\mb x@notoc\M} } } */
 /* { dg-final { scan-assembler {\mbl y\M} } } */
-/* { dg-final { scan-assembler {\mbl xx@notoc\M} } } */
+/* { dg-final { scan-assembler {\mb xx@notoc\M} } } */
+/* { dg-final { scan-assembler {\mb yy@notoc\M} } } */