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
