Re: [rs6000 PATCH] Improve constant integer multiply using rldimi.

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

[Something is wrong with your mail program.  It puts white lines
everywhere, and prefixes a space to the existing white lines.]

On Sun, Jun 26, 2022 at 09:56:07PM +0100, Roger Sayle wrote:
> It turns out this optimization already exists in the form of a combine
> splitter in rs6000.md, but the constraints on combine splitters,
> requiring three of four input instructions (and generating one or two
> output instructions) mean it doesn't get applied as often as it could.

There are no such constraints.  Or, it is more complicated, it depends
on your viewpoint :-)

Combine works on two to four related insns.  It combines those insns to
one (and one only) instruction, hopefully in canonical form.  If that
insn is not recognised by recog combine tries to split up this
instruction.  It has multiple strategies to do that.  One is to try a
define_split, which indeed is only done if there are at least three
input insns.  This is left that way even after 2->2 combines: firstly
because various backends depend on this, but also because it would cause
a lot of code movement without improvement.

> This patch converts the define_split into a define_insn_and_split to
> catch more cases (such as the one above).
>  
> The one bit that's tricky/controversial is the use of RTL's
> nonzero_bits which is accurate during the combine pass when this
> pattern is first recognized, but not as advanced (not kept up to
> date) when this pattern is eventually split.  To support this,
> I've used a "|| reload_completed" idiom.  Does this approach seem
> reasonable? [I've another patch of x86 that uses the same idiom].

No, this does not work.  All passes after combine and until reload has
completed can then get a nonzero_bits that is a subset of the one
combine saw, and thus, the insn no longer matches (as Ke Wen has
encountered.  Often you do not see this in your testing, but almost
always someone will report an ICE within days!)

[Btw, you posted the patch as quoted-printable, which means other
people can not apply the patch.]


Segher