On Fri, Dec 4, 2020 at 7:26 PM Segher Boessenkool <seg...@kernel.crashing.org> wrote: > > 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 <ja...@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.
Oh... brown paper bag time... I really don't know where and when I pick that info, since the docs indeed say: --q-- When the combiner phase tries to split an insn pattern, it is always the case that the pattern is _not_ matched by any 'define_insn'. The combiner pass first tries to split a single 'set' expression and then the same 'set' expression inside a 'parallel', but followed by a 'clobber' of a pseudo-reg to use as a scratch register. In these cases, the combiner expects exactly one or two new insn patterns to be generated. It will verify that these patterns match some 'define_insn' definitions, so you need not do this test in the 'define_split' (of course, there is no point in writing a 'define_split' that will never produce insns that match). --/q-- Enough compilers for today, I'd say. Uros.
