This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] rs6000: Use SAVE_MULTIPLE only if we restore what it saves (PR80938)


On Wed, Aug 09, 2017 at 09:28:22PM -0500, Segher Boessenkool wrote:
> On Thu, Aug 10, 2017 at 10:33:05AM +0930, Alan Modra wrote:
> > On Wed, Aug 09, 2017 at 09:06:18PM +0000, Segher Boessenkool wrote:
> > > We can have SAVE_MULTIPLE while we do not have REST_MULTIPLE.  If the
> > > inline restore does not restore all registers, the CFI for the save
> > > and restore can conflict if things are shrink-wrapped.
> > > 
> > > We could restore all registers that are saved (not ideal),
> > 
> > No, we can't do that.  A -ffixed-reg register should not be restored
> > even if it is saved, as such regs should never be written.  For
> > example, a fixed reg might be counting interrupts.  If you restore it,
> > you may lose count.  Another example is a fixed reg used as some sort
> > of context pointer.  If you restore in a function that sets a new
> > context you're obviously breaking user code.
> 
> Yes, sorry for glossing over this.  We need to handle fixed registers
> specially in most other prologue/epilogue things, too (and we hopefully
> do everywhere it is needed).

We don't.  :-(  I have a fix and will post after testing.

> > > or emit
> > > the CFI notes to say we did (which will work fine,
> > 
> > No, you can't do that either.  Unwinding might then restore a
> > -ffixed-reg register.
> 
> Yep.
> 
> > > but is also not
> > > so great); instead, let's not save the registers that are unused.
> > 
> > Ick, looks like papering over the real problem to me, and will no
> > doubt cause -Os size regressions.
> 
> I think it is very directly solving the real problem.  It isn't likely
> to cause size regressions (look how long it took for this PR to show
> up, so this cannot happen often).
> 
> This only happens if r30 (the PIC reg) is used but r31 isn't; which means
> that a) there are no other registers to save, or b) the function is marked
> as needing a hard frame pointer but eventually doesn't need one.
> 
> (RA picks the registers r31, r30, ... in sequence).
> 
> In the case in the PR, this patch replaces one insn by one (cheaper)
> insn.

And in other cases your patch will prevent stmw when it should be
used.  Testcase attached.  It shows the wrong use of lmw too.

> > Also, if SAVE_MULTIPLE causes this
> > bad interaction with shrink-wrap, does using the out-of-line register
> > save functions cause the same problem?
> 
> I do not know.  Not with the code in the PR (restoring only one or two
> registers isn't done out-of-line, and it's a sibcall exit as well).
> 
> 
> Segher

-- 
Alan Modra
Australia Development Lab, IBM
#ifdef USER_R26
register long r26 __asm__ ("r26");
#endif

int f1 (void)
{
  register long r25 __asm__ ("r25");
  register long r27 __asm__ ("r27");
  register long r28 __asm__ ("r28");
  register long r29 __asm__ ("r29");
  register long r31 __asm__ ("r31");
  __asm__ __volatile__
    ("#uses %0 %1 %2 %3 %4"
     : "=r" (r25), "=r" (r27), "=r" (r28), "=r" (r29), "=r" (r31));
  return 0;
}

void f2 (char *s)
{
  int col = 0;
  char c;

  void f3 (char c)
  {
    extern int f4 (char);
    register long r25 __asm__ ("r25");
    register long r27 __asm__ ("r27");
    register long r28 __asm__ ("r28");
    register long r29 __asm__ ("r29");
    register long r31 __asm__ ("r31");
    if (c == '\t')
      do f3 (' '); while (col % 8 != 0);
    else
      {
	__asm__ __volatile__
	  ("#uses %0 %1 %2 %3 %4"
	   : "=r" (r25), "=r" (r27), "=r" (r28), "=r" (r29), "=r" (r31));
	f4 (c);
	col++;
      }
  }

  while ((c = *s++) != 0)
    f3 (c);
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]