On Tue, Dec 03, 2019 at 01:20:04PM -0500, Michael Meissner wrote:
> On Tue, Nov 26, 2019 at 01:20:20PM -0600, Segher Boessenkool wrote:
> > > I needed to add a new constraint (em) in addition to new predicate
> > > functions.
> > > I discovered that with the predicate function alone, the register
> > > allocator
> > > would re-create the address. The constraint prevents this combination.
> >
> > It's a reasonable thing to have as a contraint, too.
> >
> > Once again, how should things work in inline assembler? Can we allow
> > prefixed addressing there, or should "m" in inline asm mean "em"?
>
> At the moment, I think we should just not allow prefixed addresses in asm
> constructs at all.
We need to think about this *before GCC 10*. And have patches for it.
So you are saying that yes, "m" in inline asm should mean "em". This
probably makes sense, there is so much existing asm that does not work
with prefixed addresses -- this is somewhet similar to how "m" in inline
asm is not the same as "m<>" (as it is in the machine description).
But then we need an extra constraint to _do_ allow prefixed addresses:
there needs to be a way to write inline asm for such memory as well.
> > > I also modified the vector extract code to generate a single PC-relative
> > > load
> > > if the vector has a PC-relative address and the offset is constant.
> >
> > That should be handled by the RTL optimisers anyway, no? Or is this
> > for post-reload splitters (a bad idea always).
>
> You have it backwards. It is the RTL optimizers that combines the vector
> extract with the load in the first place. My code allows the combination and
> generates a single instruction. Otherwise, it would do a PLA and then do the
> vector extract with the address loaded.
So, separate patch please, for a separate improvement. So that I can
understand what is what.
> > > + /* Optimize PC-relative addresses with a constant offset. */
> > > + else if (pcrel_p && CONST_INT_P (element_offset))
> > > + {
> > > + rtx addr2 = addr;
> >
> > addr isn't used after this anyway, so you can clobber it just fine.
>
> Generally I prefer not to reuse variables.
You shouldn't fear that. Instead, you should factor big routines, and/or
simplify control flow.
"Not reusing variables" (for the *same purpose* btw) is not the problem.
The problem is that the code is too big and complex and irregular to
simply understand.
> > > + /* With only one temporary base register, we can't support a
> > > PC-relative
> > > + address added to a variable offset. This is because the PADDI
> > > instruction
> > > + requires RA to be 0 when doing a PC-relative add (i.e. no register
> > > to add
> > > + to). */
> > > + else if (pcrel_p)
> > > + gcc_unreachable ();
> >
> > That comment suggests we just ICE when we get unwanted input. Such a
> > comment belongs where we prevent such code from being formed in the
> > first place (or nowhere, if it is obvious).
>
> The constraint and predicate is where we prevent it from occuring. The
> gcc_unreachable is just there as insurance that it didn't get recreated later.
> During testing before I added the constraint, I found the register allocator
> combining the two, even though the predicate didn't allow the combination. So
> the test is just to ICE out if the combination took place.
So there should be no comment here? A gcc_assert could be useful, but
that is all? And the assert should be as early as possible, in general,
like (almost) immediately after the function start. If you find you want
to do it later, congratulations, you found a good place to factor this
routine :-)
Segher
