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

Segher Boessenkool segher@kernel.crashing.org
Mon Oct 21 13:14:00 GMT 2019


On Mon, Oct 21, 2019 at 01:27:23PM +0200, Jakub Jelinek wrote:
> 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,

No, they can end up as needing reloads for hard registers in exactly the
same way as they can during combine.  The combine hook is only used by
combine, after all.

Other passes generally try only very conservative instruction modifications
(comparatively), I don't expect problems in reality, certainly not problems
that stay hidden for years.

It's just not really safe to use (non-fixed) hard registers early.


Segher



More information about the Gcc-patches mailing list