On 11/9/23 07:33, Maciej W. Rozycki wrote:
On Fri, 29 Sep 2023, Jeff Law wrote:
So this ends up looking a lot like the bits that I had to revert several weeks
ago :-)
The core issue we have is given an INSN the generic code will cost the SET_SRC
and SET_DEST and sum them. But that's far from ideal on a RISC target.
For a register destination, the cost can be determined be looking at just the
SET_SRC. Which is precisely what this patch does. When the outer code is an
INSN and we're presented with a SET we take one of two paths.
If the destination is a register, then we recurse just on the SET_SRC and
we're done. Otherwise we fall back to the existing code which sums the cost
of the SET_SRC and SET_DEST. That fallback path isn't great and probably
could be further improved (just costing SET_DEST in that case is probably
quite reasonable).
So this actually breaks insn costing for if-conversion, causing all
conditional-move expansions to count as 1 insn regardless of how many
there actually are. This can be easily verified by using various
`-mbranch-cost=' settings.
Before your change you had to set the branch cost to higher than or equal
to the replacement insn count for if-conversion to trigger. Of course
tuning microarchitectures will have preset this hopefully correctly for
their needs, so normally you don't need to resort to `-mbranch-cost='.
With this change in place only setting `-mbranch-cost=1' will prevent
if-conversion from triggering, which is taking the situation back to GCC
13 days, where `movMODEcc' patterns were always cost at 1.
In preparation for an upcoming set of changes I have written numerous
testsuite cases to verify this insn costing to work correctly and now that
I have rebased for the submission all indicate the costing went wrong and
`movMODEcc' sequences of up to 6 insns are all now cost at 1 total. I was
going to post the patch series Fri-Mon, but this seems like a showstopper
to me, because if-conversion now triggers even when the conditional-move
(or for that matter conditional-add, as I have it handled too) sequence is
more expensive than a branched one.
E.g. the NE operation costs 4 instructions for Zicond:
sub a1,a0,a1
czero.eqz a2,a2,a1
czero.nez a1,a3,a1
or a0,a2,a1
ret
while the branched equivalent costs (branch + 1) instructions:
beq a0,a1,.L3
mv a0,a2
ret
.L3:
mv a0,a3
ret
so I'd expect if-conversion only to trigger at `-mbranch-cost=3' or higher
(just as my test cases verify), but now it triggers at `-mbranch-cost=2'
already.
Can we have the insn costing reverted to correct calculation?
What needs to happen is that code needs to be extended, not reverted.
Many codes have to be synthesized based on the condition and the
true/false arms. That's not currently accounted for.
jeff
