[rs6000] Don't mark the TOC reg as set up in prologue

Revision 235876 inadvertently caused the TOC reg to be marked as set up
in prologue, which prevents shrink-wrapping from moving the prologue
past a TOC reference. The following patch corrects the situation.

Bootstrap/regtest on powerpc64le-linux and powerpc64-linux(-m32/-m64)
with no new regressions. Ok for trunk?

-Pat


2017-09-14  Pat Haugen  <pthaugen@us.ibm.com>

	* config/rs6000/rs6000.c (rs6000_set_up_by_prologue): Make sure the TOC
	reg (r2) isn't in the set of registers defined in the prologue.


testsuite/ChangeLog:
2017-09-14  Pat Haugen  <pthaugen@us.ibm.com>

	* gcc.target/powerpc/r2_shrink-wrap.c: New.

On Thu, Sep 14, 2017 at 10:18:55AM -0500, Pat Haugen wrote:
> --- gcc/config/rs6000/rs6000.c	(revision 252029)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -37807,6 +37807,11 @@ rs6000_set_up_by_prologue (struct hard_r
>      add_to_hard_reg_set (&set->set, Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
>    if (cfun->machine->split_stack_argp_used)
>      add_to_hard_reg_set (&set->set, Pmode, 12);
> +
> +  /* Make sure the hard reg set doesn't include r2, which was possibly added
> +     via PIC_OFFSET_TABLE_REGNUM.  */
> +  if (TARGET_TOC)
> +    remove_from_hard_reg_set (&set->set, Pmode, TOC_REGNUM);
>  }

Hrm, can't you simply not add it in the first place?  Just a few lines up?


Segher

[ pressed send too early ]

On Thu, Sep 14, 2017 at 10:18:55AM -0500, Pat Haugen wrote:
> --- gcc/config/rs6000/rs6000.c	(revision 252029)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -37807,6 +37807,11 @@ rs6000_set_up_by_prologue (struct hard_r
>      add_to_hard_reg_set (&set->set, Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
>    if (cfun->machine->split_stack_argp_used)
>      add_to_hard_reg_set (&set->set, Pmode, 12);
> +
> +  /* Make sure the hard reg set doesn't include r2, which was possibly added
> +     via PIC_OFFSET_TABLE_REGNUM.  */
> +  if (TARGET_TOC)
> +    remove_from_hard_reg_set (&set->set, Pmode, TOC_REGNUM);
>  }

And why is the problem in PR51872 no longer there?  Or is it?


Segher

On 09/14/2017 11:35 AM, Segher Boessenkool wrote:
> On Thu, Sep 14, 2017 at 10:18:55AM -0500, Pat Haugen wrote:
>> --- gcc/config/rs6000/rs6000.c	(revision 252029)
>> +++ gcc/config/rs6000/rs6000.c	(working copy)
>> @@ -37807,6 +37807,11 @@ rs6000_set_up_by_prologue (struct hard_r
>>      add_to_hard_reg_set (&set->set, Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
>>    if (cfun->machine->split_stack_argp_used)
>>      add_to_hard_reg_set (&set->set, Pmode, 12);
>> +
>> +  /* Make sure the hard reg set doesn't include r2, which was possibly added
>> +     via PIC_OFFSET_TABLE_REGNUM.  */
>> +  if (TARGET_TOC)
>> +    remove_from_hard_reg_set (&set->set, Pmode, TOC_REGNUM);
>>  }
> Hrm, can't you simply not add it in the first place?  Just a few lines up?

As we discussed offline, that is RS6000_PIC_OFFSET_TABLE_REGNUM (i.e.
r30). PIC_OFFSET_REGNUM is added in shrink-wrap.c:try_shrink_wrapping().

I noticed that the test fails on 32-bit because there's no prologue and
hence no shrink-wrapping to be performed. Following is updated testcase
which limits it to 64-bit. Ok?

-Pat

Index: gcc/testsuite/gcc.target/powerpc/r2_shrink-wrap.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/r2_shrink-wrap.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/r2_shrink-wrap.c	(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { lp64 } } } */
+/* { dg-options "-O2 -fdump-rtl-pro_and_epilogue" } */
+
+/* Verify we move the prologue past the TOC reference of 'j' and
shrink-wrap
+   the function. */
+void bar();
+int j;
+void foo(int i)
+{
+  j = i;
+  if (i > 0)
+    {
+      bar();
+    }
+}
+
+/* { dg-final { scan-rtl-dump-times "Performing shrink-wrapping" 1
"pro_and_epilogue" } } */

On Thu, Sep 14, 2017 at 11:53:02AM -0500, Pat Haugen wrote:
> On 09/14/2017 11:35 AM, Segher Boessenkool wrote:
> > On Thu, Sep 14, 2017 at 10:18:55AM -0500, Pat Haugen wrote:
> >> --- gcc/config/rs6000/rs6000.c	(revision 252029)
> >> +++ gcc/config/rs6000/rs6000.c	(working copy)
> >> @@ -37807,6 +37807,11 @@ rs6000_set_up_by_prologue (struct hard_r
> >>      add_to_hard_reg_set (&set->set, Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
> >>    if (cfun->machine->split_stack_argp_used)
> >>      add_to_hard_reg_set (&set->set, Pmode, 12);
> >> +
> >> +  /* Make sure the hard reg set doesn't include r2, which was possibly added
> >> +     via PIC_OFFSET_TABLE_REGNUM.  */
> >> +  if (TARGET_TOC)
> >> +    remove_from_hard_reg_set (&set->set, Pmode, TOC_REGNUM);
> >>  }
> > Hrm, can't you simply not add it in the first place?  Just a few lines up?
> 
> As we discussed offline, that is RS6000_PIC_OFFSET_TABLE_REGNUM (i.e.
> r30). PIC_OFFSET_REGNUM is added in shrink-wrap.c:try_shrink_wrapping().

Ah, right, I never can keep the two apart.

> I noticed that the test fails on 32-bit because there's no prologue and
> hence no shrink-wrapping to be performed. Following is updated testcase
> which limits it to 64-bit. Ok?

> --- gcc/testsuite/gcc.target/powerpc/r2_shrink-wrap.c	(nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/r2_shrink-wrap.c	(working copy)
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target { lp64 } } } */

You don't need the braces around lp64.

> +/* { dg-options "-O2 -fdump-rtl-pro_and_epilogue" } */
> +
> +/* Verify we move the prologue past the TOC reference of 'j' and
> shrink-wrap
> +   the function. */
> +void bar();
> +int j;
> +void foo(int i)
> +{
> +  j = i;
> +  if (i > 0)
> +    {
> +      bar();

If you add   asm ("");   here, it won't do a sibcall on any (sub-)target.
Not that this testcase is relevant elsewhere anyway.

> +    }
> +}
> +
> +/* { dg-final { scan-rtl-dump-times "Performing shrink-wrapping" 1
> "pro_and_epilogue" } } */

Okay for trunk, with or without that asm.  Thanks!


Segher

On Thu, Sep 14, 2017 at 11:39:54AM -0500, Segher Boessenkool wrote:
> [ pressed send too early ]
> 
> On Thu, Sep 14, 2017 at 10:18:55AM -0500, Pat Haugen wrote:
> > --- gcc/config/rs6000/rs6000.c	(revision 252029)
> > +++ gcc/config/rs6000/rs6000.c	(working copy)
> > @@ -37807,6 +37807,11 @@ rs6000_set_up_by_prologue (struct hard_r
> >      add_to_hard_reg_set (&set->set, Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);
> >    if (cfun->machine->split_stack_argp_used)
> >      add_to_hard_reg_set (&set->set, Pmode, 12);
> > +
> > +  /* Make sure the hard reg set doesn't include r2, which was possibly added
> > +     via PIC_OFFSET_TABLE_REGNUM.  */
> > +  if (TARGET_TOC)
> > +    remove_from_hard_reg_set (&set->set, Pmode, TOC_REGNUM);
> >  }
> 
> And why is the problem in PR51872 no longer there?  Or is it?

This code in rs6000_set_up_by_prologue:

  if (!TARGET_SINGLE_PIC_BASE
      && TARGET_TOC
      && TARGET_MINIMAL_TOC
      && !constant_pool_empty_p ())
    add_to_hard_reg_set (&set->set, Pmode, RS6000_PIC_OFFSET_TABLE_REGNUM);

adds r30, the -mminimal-toc toc pointer register set up by the
prologue.  Pat's change removes r2.  Which looks correct to me.