%s/format/form/g
On Tue, Jul 16, 2019 at 01:29:50PM -0400, Michael Meissner wrote:
> On Fri, Jul 12, 2019 at 11:49:51AM -0500, Segher Boessenkool wrote:
> > Many of those are not clear why you allow or do not allow certain forms,
> > or what each is meant to be used for. For example:
> >
> > > + enum rs6000_offset_format format_gpr_64bit
> > > + = (TARGET_POWERPC64) ? OFFSET_FORMAT_DS : OFFSET_FORMAT_D;
> >
> > Why does non-powerpc64 allow D? It cannot even do 64-bit GPR accesses
> > *at all*!
>
> A single instruction cannot do 64-bit GPR loads, BUT the tests are needed for
> register allocation before the instruction is split. So in 32-bit mode, load
> of DImode is split into 2 loads of SImode, which is D-format.
Then the variable is misnamed?
> > > + enum rs6000_offset_format format_gpr_128bit
> > > + = (TARGET_QUAD_MEMORY) ? OFFSET_FORMAT_DQ : format_gpr_64bit;
> >
> > 128-bit GPRs do not exist, either.
>
> Yes they do during register allocation, and are split later before final.
You mean that you have a TImode datum in two GPRs. There are no 128-bit
GPRs.
The *two* GPRs should be part of the name, that would improve it?
> This is in part why I think the mode format is broken.
The alternative is picking at expand time what register set to use where.
That is worse. We want to use vectors often, but certainly not always.
> But then I've felt that the current legitimate address stuff has been broken
> ever since I started working on GCC 1.37 because you don't have the context of
> whether it is a load or store, and don't have the register loaded to or stored
> after register allocation. But it is what it is.
That is one of reload's big jobs. If you have a better design... :-)
> > > + /* Small integers. */
> > > + reg_addr[E_QImode].offset_format = OFFSET_FORMAT_D;
> > > + reg_addr[E_HImode].offset_format = OFFSET_FORMAT_D;
> > > + reg_addr[E_CQImode].offset_format = OFFSET_FORMAT_D;
> > > + reg_addr[E_CHImode].offset_format = OFFSET_FORMAT_D;
> >
> > Do we handle CQI and CHI anywhere else?
>
> It depends. It may use this format early in the RTL generation until the
> complex types are split.
Is that used *during* expand? Blergh.
> > > + /* While DImode can be loaded into a FPR register with LFD (d-form),
> > > the
> > > + normal use is to use a GPR (ds-form in 64-bit). */
> > > + reg_addr[E_DImode].offset_format = format_gpr_64bit;
> > > + reg_addr[E_CDImode].offset_format = format_gpr_64bit;
> >
> > I'm not sure that comment helps anything -- I don't know what it is trying
> > to say?
>
> Normally when you use a DImode, it goes into a GPR (i.e. DS-format in 64-bit).
> However, there are times when you want to use a DImode in a FPR, when you can
> use D-format. As an example, the test dimode_off.c hand crafts loads and
> stores with odd offsets. We had to de-tune movdi so that the register
> allocator would not load the value into a FPR and do a direct move (or do a
> direct move and store from the FPR).
That doesn't explain the comment to me, sorry :-/
> > > + /* Condition registers (uses LWZ/STW). */
> > > + reg_addr[E_CCmode].offset_format = OFFSET_FORMAT_D;
> > > + reg_addr[E_CCUNSmode].offset_format = OFFSET_FORMAT_D;
> > > + reg_addr[E_CCFPmode].offset_format = OFFSET_FORMAT_D;
> > > + reg_addr[E_CCEQmode].offset_format = OFFSET_FORMAT_D;
> >
> > Okay, why do we make the "allowed address form" function support modes
> > that we have no load/store insns for?!
>
> But we do have load and store functions for the CC* modes. See
> movcc_internal1.
I meant machine instructions.
movcc_internal has nothing to load/store anything in a CC reg from/to
memory, either.
Do we really have to care what might happen *after* we put something in
a GPR?
> > > + /* 128-bit integers. */
> > > + if (TARGET_POWERPC64)
> > > + {
> > > + reg_addr[E_TImode].offset_format = format_vector;
> >
> > TI is not normally / always / often in vectors.
>
> Not now, but it will be.
1) Erm.
2) No, you cannot break p8 and p9 performance like this.
> Basically the whole thing is a guess of what the valid offset format for a
> mode
> is. This is needed for the legitimate address functions
> (i.e. rs6000_legitimate_address_p, rs6000_legitimate_offset_address_p) to
> validate whether we allow odd addresses or not.
This needs to be made much more explicit, then. "preferred_form" or
something.
Then suddenly it will probably start to make sense :-)
Segher
