[RS6000] Rewrite rs6000_frame_related to use simplify_replace_rtx

[Segher Boessenkool]

On Thu, May 05, 2016 at 06:49:04AM +0930, Alan Modra wrote:
> >  And it's a better name anyway?
> 
> No, "real" seems silly to me.  "patt" is a common idiom used in lots
> of places for the pattern of an instruction.

"patt" is used only once (in fwprop), everything else uses "pat".

> What is "real" supposed
> to mean?  The real pattern vs. some imaginary one?

Yes exactly.  This function is making a note that does the same thing
as the real insn.

> The final pattern
> we want?  The last meaning might have made some sense in the very
> first implementation of rs6000_frame_related where the code did
> something like
>   real = replace_rtx (PATTERN (insn), ...);
> then made simplify_rtx calls.
> 
> I think "real" is confusing when we're making substitutions step by
> step.

True enough, okay, the later part of the function changes "real".

> > > -  if (REGNO (reg) == STACK_POINTER_REGNUM && reg2 == NULL_RTX)
> > > +  repl = NULL_RTX;
> > > +  if (REGNO (reg) == STACK_POINTER_REGNUM)
> > > +    gcc_checking_assert (val == 0);
> > > +  else
> > > +    repl = gen_rtx_PLUS (Pmode, gen_rtx_REG (Pmode, STACK_POINTER_REGNUM),
> > > +			 GEN_INT (val));
> > 
> > Put the NULL_RTX assignment in the first arm, please.
> 
> OK, I'll make that style change, but only because we have that
> gcc_checking_assert there.
> 
> Otherwise I would have written
>   repl = NULL_RTX;
>   if (REGNO (reg) != STACK_POINTER_REGNUM)
>     repl = gen_rtx_PLUS (Pmode, gen_rtx_REG (Pmode, STACK_POINTER_REGNUM),
> 			 GEN_INT (val));
> 
> which is better than
>   if (REGNO (reg) == STACK_POINTER_REGNUM)
>     repl = NULL_RTX;
>   else
>     repl = gen_rtx_PLUS (Pmode, gen_rtx_REG (Pmode, STACK_POINTER_REGNUM),
> 			 GEN_INT (val));

I think we're supposed to use a ? : thing for that.  Sucks as well; I would
try to rewrite the whole code to avoid such nasties.  Or move on :-)


Segher