[PATCH] Fix (hypothetical) problem with pre-reload splitters (PR target/92140)

Jakub Jelinek jakub@redhat.com
Mon Oct 21 11:31:00 GMT 2019


On Mon, Oct 21, 2019 at 05:45:16AM -0500, Segher Boessenkool wrote:
> On Sun, Oct 20, 2019 at 09:51:22PM +0200, Uros Bizjak wrote:
> > On Sun, Oct 20, 2019 at 1:24 PM Jakub Jelinek <jakub@redhat.com> wrote:
> > > As mentioned in the PR, the x86 backend has various define_insn_and_split
> > > patterns that are meant to match usually during combine, are then
> > > unconditionally split during split1 pass and as they have && can_create_pseudo_p ()
> > > in their define_insn condition, if they get matched after split1, nothing
> > > would split them anymore and they wouldn't match after reload.
> > >
> > > The split1 pass already sets a property that can be used.
> > >
> > > I've first tried to remove some constraints and associated attributes, but
> > > it seems from further discussions in the PR I don't know much about the
> > > reasons why they were added and if they are still needed or not, so this
> > > version of the patch just replaces the can_create_pseudo_p () conditions
> > > with a new predicate that stops matching already after the split1 pass.
> > 
> > As explained by Segher in the PR, there is no 100% guarantee that
> > combine won't produce a pattern with a wrong hard register as a e.g.
> > count reg of a shift insn. RA will die on this kind of pattern with
> > reload failure, so these constraints are used together with
> > ix86_legitimate_combined_insn target hook to reject invalid
> > combinations involving hard registers.
> 
> And even that isn't completely safe, there is nothing that stops other
> passes from creating the offending insns.

Sure, but with the patch either they are created before split1, then they
will be split there and all is fine, or they are created after split1 and
will not match.  The only problematic case would be if they are created
during split1 when splitting other insns, not really sure if split1 recurses
on the newly created insns, but even if it doesn't, I'd say it would be a
backend bug that it created them and I strongly doubt i386 backend does
that.

	Jakub



More information about the Gcc-patches mailing list