rs6000: Entry point generation for functions using PC-relative addressing

Message ID 2feecb0e-7ec9-1f3c-dfd4-6b40c4c5f518@linux.ibm.com
State New
Headers show
Series
  • rs6000: Entry point generation for functions using PC-relative addressing
Related show

Commit Message

Bill Schmidt May 22, 2019, 11:39 p.m.
Hi,

Functions using PC-relative addressing do not use a TOC, so there is no need for
a global entry point for TOC setup.  Ensure we never generate a global entry
point, and use a .localentry directive that sets the upper st_other bits to 1.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
Is this okay for trunk?

Thanks,
Bill


[gcc]

2019-05-22  Bill Schmidt  <wschmidt@linux.ibm.com>

	* config/rs6000/rs6000.c (rs6000_global_entry_point_needed_p):
	Return false for PC-relative functions.
	(rs6000_output_function_prologue): Emit ".localentry name,1" for
	PC-relative functions.

[gcc/testsuite]

2019-05-22  Bill Schmidt  <wschmidt@linux.ibm.com>

	* gcc.target/powerpc/localentry-1.c: New file.

Comments

Segher Boessenkool May 23, 2019, 11:06 a.m. | #1
Hi!

On Wed, May 22, 2019 at 06:39:55PM -0500, Bill Schmidt wrote:
> @@ -26191,6 +26191,10 @@ rs6000_global_entry_point_needed_p (void)

>    if (TARGET_SINGLE_PIC_BASE)

>      return false;

>  

> +  /* PC-relative functions never generate a global entry point prologue.  */

> +  if (rs6000_pcrel_p (cfun))

> +    return false;


"global_entry_point_needed" is such a confusing name; it isn't what this
function is about.  "global entry point prologue" like in your comment
is much closer to the truth, but also not exactly it.

Maybe we should rename this function.  And/or split it into two.

> +  const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);


This var is only used on some code paths.  Factor this better, instead?
This ties in with the previous point.

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/powerpc/localentry-1.c

> @@ -0,0 +1,19 @@

> +/* { dg-do compile } */

> +/* { dg-options "-mcpu=future -O2" } */

> +/* { dg-require-effective-target powerpc_elfv2 } */

> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=future" } } */


Lose this last line, use -mdejagnu-cpu=future instead please.


Segher
Bill Schmidt May 23, 2019, 1:56 p.m. | #2
On 5/23/19 6:06 AM, Segher Boessenkool wrote:
> Hi!

>

> On Wed, May 22, 2019 at 06:39:55PM -0500, Bill Schmidt wrote:

>> @@ -26191,6 +26191,10 @@ rs6000_global_entry_point_needed_p (void)

>>    if (TARGET_SINGLE_PIC_BASE)

>>      return false;

>>  

>> +  /* PC-relative functions never generate a global entry point prologue.  */

>> +  if (rs6000_pcrel_p (cfun))

>> +    return false;

> "global_entry_point_needed" is such a confusing name; it isn't what this

> function is about.  "global entry point prologue" like in your comment

> is much closer to the truth, but also not exactly it.

>

> Maybe we should rename this function.  And/or split it into two.

I think "global entry point prologue" is exactly right, and the
function-level comment even says that.  So we could change this to
rs6000_global_entry_point_prologue_needed_p.  A bit chewy but digestible.
>

>> +  const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);

> This var is only used on some code paths.  Factor this better, instead?

> This ties in with the previous point.

>

>> --- /dev/null

>> +++ b/gcc/testsuite/gcc.target/powerpc/localentry-1.c

>> @@ -0,0 +1,19 @@

>> +/* { dg-do compile } */

>> +/* { dg-options "-mcpu=future -O2" } */

>> +/* { dg-require-effective-target powerpc_elfv2 } */

>> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=future" } } */

> Lose this last line, use -mdejagnu-cpu=future instead please.


Yeah, sorry, original patch predated that change and I missed it again.

Thanks!
Bill
>

>

> Segher

>
Segher Boessenkool May 23, 2019, 3:15 p.m. | #3
On Thu, May 23, 2019 at 08:56:23AM -0500, Bill Schmidt wrote:
> On 5/23/19 6:06 AM, Segher Boessenkool wrote:

> > On Wed, May 22, 2019 at 06:39:55PM -0500, Bill Schmidt wrote:

> >> @@ -26191,6 +26191,10 @@ rs6000_global_entry_point_needed_p (void)

> >>    if (TARGET_SINGLE_PIC_BASE)

> >>      return false;

> >>  

> >> +  /* PC-relative functions never generate a global entry point prologue.  */

> >> +  if (rs6000_pcrel_p (cfun))

> >> +    return false;

> > "global_entry_point_needed" is such a confusing name; it isn't what this

> > function is about.  "global entry point prologue" like in your comment

> > is much closer to the truth, but also not exactly it.

> >

> > Maybe we should rename this function.  And/or split it into two.

> I think "global entry point prologue" is exactly right, and the

> function-level comment even says that.  So we could change this to

> rs6000_global_entry_point_prologue_needed_p.  A bit chewy but digestible.


So I see the second use really needs the same condition...  okay.

What it checks is if the function needs a local entry point not equal to
the global entry point.

Anyway...  Not too important, the function isn't used in many places at all.


Segher

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 959e222c9cb..23c6d37ab55 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -26191,6 +26191,10 @@  rs6000_global_entry_point_needed_p (void)
   if (TARGET_SINGLE_PIC_BASE)
     return false;
 
+  /* PC-relative functions never generate a global entry point prologue.  */
+  if (rs6000_pcrel_p (cfun))
+    return false;
+
   /* Ensure we have a global entry point for thunks.   ??? We could
      avoid that if the target routine doesn't need a global entry point,
      but we do not know whether this is the case at this point.  */
@@ -27545,12 +27549,12 @@  rs6000_output_function_prologue (FILE *file)
 #endif
     }
 
+  const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
+
   /* ELFv2 ABI r2 setup code and local entry point.  This must follow
      immediately after the global entry point label.  */
   if (rs6000_global_entry_point_needed_p ())
     {
-      const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
-
       (*targetm.asm_out.internal_label) (file, "LCF", rs6000_pic_labelno);
 
       if (TARGET_CMODEL != CMODEL_LARGE)
@@ -27601,6 +27605,18 @@  rs6000_output_function_prologue (FILE *file)
       fputs ("\n", file);
     }
 
+  else if (rs6000_pcrel_p (cfun))
+    {
+      /* All functions compiled to use PC-relative addressing will
+	 have a .localentry value of 0 or 1.  For now we set it to
+	 1 all the time, indicating that the function may clobber
+	 the TOC register r2.  Later we may optimize this by setting
+	 it to 0 if the function is a leaf and does not clobber r2.  */
+      fputs ("\t.localentry\t", file);
+      assemble_name (file, name);
+      fputs (",1\n", file);
+    }
+
   /* Output -mprofile-kernel code.  This needs to be done here instead of
      in output_function_profile since it must go after the ELFv2 ABI
      local entry point.  */
diff --git a/gcc/testsuite/gcc.target/powerpc/localentry-1.c b/gcc/testsuite/gcc.target/powerpc/localentry-1.c
new file mode 100644
index 00000000000..15e00d9917e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/localentry-1.c
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=future -O2" } */
+/* { dg-require-effective-target powerpc_elfv2 } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=future" } } */
+
+/* Ensure we generate ".localentry fn,1" for both leaf and non-leaf
+   functions.  */
+
+extern int y (int);
+
+int x (void)
+{
+  return y (5);
+}
+
+void z (void) { };
+
+/* { dg-final { scan-assembler {\.localentry\t\mx,1\M} } } */
+/* { dg-final { scan-assembler {\.localentry\t\mz,1\M} } } */