Hi Mike,
On Mon, Nov 25, 2019 at 05:09:10PM -0500, Michael Meissner wrote:
> On Fri, Nov 22, 2019 at 06:06:19PM -0600, Segher Boessenkool wrote:
> > > --- gcc/config/rs6000/rs6000.c (revision 278173)
> > > +++ gcc/config/rs6000/rs6000.c (working copy)
> > > @@ -5552,7 +5552,7 @@ static int
> > > num_insns_constant_gpr (HOST_WIDE_INT value)
> > > {
> > > /* signed constant loadable with addi */
> > > - if (((unsigned HOST_WIDE_INT) value + 0x8000) < 0x10000)
> > > + if (SIGNED_16BIT_OFFSET_P (value))
> > > return 1;
> >
> > Hrm, so the SIGNED_nBIT_OFFSET_P should not be called "offset", if we use
> > them for other numbers as well.
Please do something about this? Not in this patch, of course.
> Is this what you want?
Yeah, like that.
Please send a patch doing *only* this typographical change -- no code
changes at all -- before patches that change the code. Or, alternatively,
just split up lines that you are touching anyway, and eventually we will
get something nice.
Either option is much easier to review than patches moving 60 constraints
around :-)
This is general advice: if you want to rearrange some code, or fix (some
non-trivial) white space, etc., do that in a separate patch *before* the
rest. Same as with bugfixes.
That way, it can be immediately applied, whether the rest of the patches
needs further revisions or not. And it makes reviewing the rest a lot
easier too.
> Now, this is without moving any of the alternatives around. Logically, you
> could move gpr/fpr/vsx moves to be together (and maybe the direct moves also).
> But I dunno whether this is getting into bike shedding.
Yeah you cannot always move things the way you like them, the ordering
also matters for RA itself (sometimes). It's all not a big deal except
we have so very very many alternatives in the mov patterns.
Segher
