On 19/10/2019 17:17, Segher Boessenkool wrote:
On Fri, Oct 18, 2019 at 08:48:42PM +0100, Richard Earnshaw wrote:
Consider this sequence during combine:
Trying 18, 7 -> 22:
18: r118:SI=r122:SI
REG_DEAD r122:SI
7: r114:SI=0x1-r118:SI-ltu(cc:CC_RSB,0)
REG_DEAD r118:SI
REG_DEAD cc:CC_RSB
22: r1:SI=r114:SI
REG_DEAD r114:SI
r122 is dead here. combine will have tried just 7+22 before, and that
failed; it now only succeeds because the register copy is removed.
All like you say and what your patch is about. But, this is a generic
problem, that should be solved in combine itself (whether or not you also
want to reduce the insn cost here).
This patch can easily be reverted if combine is changed; it doesn't
really interfere with the other patches in the series.
My real problem was that this case didn't really show up until I
introduced other patches in this series and this combine simplification
was then causing the generated code to be significantly worse.
Maybe we should simply disallow pseudo-to-pseudo (or even all) copies when
combining more than two insns, always? I'll experiment.
We don't want to prevent combine from eliminating such moves, as this
can expose more combine opportunities, but we shouldn't rate them as
profitable in themselves. We can do this be adjusting the costs
slightly so that the benefit of eliminating such a simple insn is
reduced.
Yes, but combine should have removed the move in a 2->1 combination
already, if it is beneficial: both 18->7 and 7->22 should have combined
just fine. This also points to a potential target problem: why are those
two combinations not allowed?
In this case it's because we have a generic pattern for
reg = const - reg - ltu(ccreg)
and the specific case of const == 1, which is then re-ordered as
reg = (1 - ltu(ccreg)) - reg
=> reg = geu(ccreg) - reg
and we don't have a pattern for that. It would be possible to write one
if really needed, but because the GEU has to match several different
modes, it's a bit fiddly and results in a lot of code for not much real
benefit.
For the 2-insn case we don't try a split and simply give up; but when we
have a three-insn sequence we then combine tries harder and the generic
splitter does rewrite that in 2 insns.
Perhaps combine should never try a 3, or 4 insn combination where i0 or
i1 is a simple reg-reg move and only feeds one subsequent insn in the
sequence on the basis that if that was really going to help then it
would have been caught by the two-insn sequence. If it feeds both i2
and i3 then that's a different matter, of course.
R.
