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
