On 11/6/25 10:13 AM, Palmer Dabbelt wrote:
On Wed, 05 Nov 2025 18:31:36 PST (-0800), Jeff Law wrote:
Let's consider this code fragment (from xalan):
[snip]
Then we realized we really should just ignore the (set (reg) (const_int
0)) in the if-converted sequence. We're going to be able to propagate
that away in nearly every case since we have a hard-wired zero register.
Sure enough, ignoring that insn was enough to tip the balance on this
case and we get the desired code.
Seems like maybe we should be doing that everywhere? I think we're
doing it for RTX costs (the SET is free and then the immediate costs to
0). I don't think we're doing it for the insn_cost case, though --
unless something else is happening somewhere that I don't understand.
I briefly opened up the ifcvt code, saw it calling both RTX and insn
costs, and then decided it'd be better to just say something here rather
than try to figure ouf it changing insn_cost would help this specific
case ;)
It's largely a question of where/when it shows up in the IL.
As an operand a (const_int 0) should be equivalent to a register for
most integer cases. It's neither better nor worse.
An insn that does (set (reg) (const_int 0)) in general may or may not
have a cost -- consider arguments, return values, initialization of loop
counters, etc. So in the absence of additional information I'd be
inclined to leave them as-is barring a testcase where costing is a problem.
For if-conversion we've generated a little sequence and we're going to
cost that sequence by summing up the cost of every insn in the sequence.
Expectation is a (set (reg) (const_int 0)) showing up in that sequence
is most likely related to the sequence itself rather than something like
setting up function args, return values, etc. So ignoring it in that
context seems reasonable to me.
So I think we're reasonable as a whole. Maybe not perfect, but I'd want
to see actual testcodes where it mattered before making a wider scoped
change.
WRT the actual patch. Testcase was missing "b" in its -march string
causing failures and there's a whitespace goof in the riscv.cc change.
I'll be fixing those and pushing shortly.
jeff
