Hi!

On Sun, Sep 19, 2021 at 09:14:55AM -0600, Jeff Law wrote:
> On 9/6/2021 8:24 AM, Segher Boessenkool wrote:
> >On Mon, Sep 06, 2021 at 12:32:13PM +0100, Roger Sayle wrote:
> >>I think the current documentation is sufficient.  During compilation, 
> >>GCC's
> >>combine pass will often substitute a register with an expression defining
> >>it's value, and then attempt to simplify it.  As you point out, this often
> >>produces
> >>(temporary intermediate) expressions with poorly defined semantics.
> >Not "poorly defined".  The documentation says (in rtl.texi)
> >
> >@findex subreg
> >@item (subreg:@var{m1} @var{reg:m2} @var{bytenum})
> >
> >@code{subreg} expressions are used to refer to a register in a machine
> >mode other than its natural one, or to refer to one register of
> >a multi-part @code{reg} that actually refers to several registers.
> >
> >It goes on to say there also are subregs of mem currently; it
> >exhaustively lists all things you can take a subreg of, what the
> >different semantics of those are, how the semantics are further
> >"modified" (read: completely different) if some RTL flags are set, etc.
> >
> >The instruction combiner very often creates invalid RTL (only
> >structurally valid, like, no missing operands).  Invalid RTL is supposed
> >to never match (because backends will not have patterns that match
> >these).  combine even creates invalid RTL on purpose (a clobber of
> >const0_rtx) to ensure its result is deemed invalid, when it wants to
> >abort a combination attempt for some reason.
> >
> >>The
> >>purpose of my patch is to avoid (constructing) SUBREGs that have TRUNCATE
> >>as an argument (that some folks consider undefined) and instead simplify
> >>them to either a single TRUNCATE or a SUBREG of a REG, both of which are
> >>well defined.  I'm making/helping the implementation match the
> >>documentation.
> >But this should never make it to simplify-rtx at all.  You can only
> >validly do such transformations in combine, because you need to know
> >what insns you started with etc.
> >
> >simplify-rtx is a part of combine, sure, but it is used from other
> >contexts as well nowadays.  If you can safely do this from combine,
> >you can do it in combine.
> [ ... ]
> So what I think is missing here is some concrete path forward.  If I'm 
> understanding your objection Segher, you'd like to see Roger look at 
> where these (subreg (truncate)) expressions came from and address them 
> earlier than simplify_rtx?

There is no such thing as "earlier than simplify-rtx", that is the
point.  simplify-rtx is not a pass: it is like a library that is used
from all over the RTL routines.

If something belongs in combine, it should be in combine, not in
simplify-rtx.

> The theory being that the simplification 
> bits could be used from other contexts than just combine and in those 
> other contexts (subreg (truncate)) isn't valid?

Or not actually simplifying (or canonicalising).  Yup.

It also is the case that invalid RTL should simply never be constructed,
we should not (try to) fix it up later.  I'm not sure if that was what
was happening in this case, too much of this stuff recently, everything
blurs together, sorry :-/

I would love to see this all improved.  But if we just poke at it with
a stick we'll be in worse shape instead of better by the time stage 1
closes.  So we need a plan, and/or a topic tree (some devel/* branch or
whatever).


Segher

Reply via email to