On Wed, May 04, 2016 at 11:26:18AM -0500, Segher Boessenkool wrote:
> On Wed, May 04, 2016 at 11:14:41AM +0930, Alan Modra wrote:
> > * config/rs6000/rs6000.c (rs6000_frame_related): Rewrite.
>
> > - rtx real, temp;
> > + rtx patt, repl;
>
> If you don't rename "real" here it is probably easier to read?
Easier to read the diff, yes.
> 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. What is "real" supposed
to mean? The real pattern vs. some imaginary one? 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.
> > - 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));
--
Alan Modra
Australia Development Lab, IBM
