On Tue, Sep 21, 2021 at 6:18 AM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 9/20/2021 6:23 PM, Segher Boessenkool wrote:
> > 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.
> I'm referring to earlier in the call chain, not an earlier pass. Sorry I
> wasn't clear about that.
>
> If we were catching the scenario which led to the creation of (subreg
> (truncate)) in combine and instead of creating (subreg (truncate)) we
> instead created the simplified, correct form would that ease your concerns?
Sorry to chime in, but if (subreg (truncate ...)) is invalid it would be OK for
simplify_gen_subreg () to
1) return NULL_RTX
2) return valid RTX, in this case the suggested canoncalized form
I agree that simplify_gen_subreg should not return (subreg (truncate ..)).
So I'm not understanding if you are suggesting that even calling
simplify_gen_subreg on a (truncate ...) is invalid?
Now, the patch changes simplify_subreg where it's not clear whether
the subreg already exists (it should not) or whether we are about to
construct it, seeing if there's a valid RTX that expresses the semantic
as if the op were pushed to a pseudo (aka we're combining on the fly).
It's a bit like fold_convert (double_type_node, integer_one_node)
second-guessing and doing a FLOAT_EXPR when folding
the implicit CONVERT_EXPR.
Richard.
>
>
> Jeff
