Re: [PATCH], Tweak PowerPC movdi constraints

[Segher Boessenkool <segher at kernel dot crashing dot org>]

On Fri, Nov 18, 2016 at 05:52:12PM -0500, Michael Meissner wrote:
> On Fri, Nov 18, 2016 at 04:43:40PM -0600, Segher Boessenkool wrote:
> > On Fri, Nov 18, 2016 at 03:38:38PM -0500, Michael Meissner wrote:
> > > This patch tweaks the movdi constraints for the PowerPC to use "^" or "$"
> > > constraints instead of "?*".  This allows the register allocator to more often
> > > allocate DImode to a floating point/vector register when it is desirable to do
> > > so.
> > 
> > It also changes some plain "?" to "^" or "$" or even "*" (which cannot
> > work for multi-character constraints, it skips one character, not one
> > constraint).  Wrong version of the patch?
> 
> Note, if '*' does not work with multi-character prefixes, that is a bug.  All
> of rs6000.md assumes that ?*wa means that the register allocator will not
> consider VSX vector registers for when calculating register preferences.

The documentation is out of date.

>From ira-costs.c:

          /* Scan all the constraint letters.  See if the operand
             matches any of the constraints.  Collect the valid
             register classes and see if this operand accepts
             memory.  */
          while ((c = *p))
            {
              switch (c)
                {
                case '*':
                  /* Ignore the next letter for this pass.  */
                  c = *++p;
                  break;

and then 83 lines later:

              p += CONSTRAINT_LEN (c, p);
              if (c == ',')
                break;
            }

so it does in fact work.

Neither the patch description nor the changelog says you are doing these
changes though.

> > > I built bootstrap compilers and did make check with no regressions on:
> > >     1)	Little endian power8, --with-cpu=power8
> > >     2)	Big endian power8, --with-cpu=power8 (no 32-bit support)
> > >     3)	Big endian power7, --with-cpu=power7 (both 32/64-bit support)
> > 
> > Could you also test with reload please?  Just LE is enough I guess.
> > We'd like to keep reload working for GCC 7 at least, and these cost
> > prefixes tend to break mov patterns :-/
> 
> Argh, I guess you are right, but then if reload doesn't work, I will likely
> submit the patch where there are three different movdi's (one for 32-bit
> without the change, one for 64-bit with reload, and one for 64-bit with lra).
> I would prefer not to do that.

Let's hope it just works :-)


Segher