[PATCH] i386: Add combine splitters to allow combining multiple insns into reg1 = const; reg2 = rotate (reg1, reg3 & cst) [PR96226]

[Segher Boessenkool]

Hi!

On Fri, Dec 04, 2020 at 07:06:45PM +0100, Uros Bizjak wrote:
> On Fri, Dec 4, 2020 at 6:57 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Fri, Dec 04, 2020 at 06:53:49PM +0100, Uros Bizjak wrote:
> > > > > I was trying that first, but it didn't work.  Without the
> > > > > clobber it actually works right, we don't have the rotate insn with the
> > > > > masking and no clobber, so in the end combiner does add the clobber there
> > > > > (or would fail it the clobber couldn't be added).
> > > >
> > > > I was not aware of that detail ...
> > >
> > > That said, IMO, it would be better to rewrite other _mask and _mask_1
> > > patterns that remove useless masking to combine splitter.
> > > Unfortunately, the combine splitter expects exactly two output
> > > instructions for some reason, but these patterns split to one
> > > instruction. Perhaps it is possible to relax this limitation of
> > > combine splitters and also allow one output instruction.
> >
> > I've already checked it in.  Guess I can try to change the combine splitters
> > (can it wait till Monday?) so that they remove the masking when splitting
> > the insn into two, so that the pre-reload splitters aren't involved.
> 
> No, I didn't want to burden you with the additional task - the patch
> is OK as it is. I was just thinking out loud, as I remembered that
> changing bt patterns to combine splitter regressed one testcase. IIRC
> combination of two insns blocked better combination of three insns, or
> something like that.
> 
> > To turn those pre-reload define_insn_and_splits I'm afraid we'd indeed
> > need combiner's changes, so that would need to be discussed with Segher
> > first.
> 
> Yes, that is the long-term plan. Segher CC'd.

A splitter can *already* split to only one insn.


Segher