jameysharp commented on issue #6086:
Neat, thanks for picking this up! I think this is correct, but I'd like @elliottt or @abrown to take a look.
meithecatte commented on issue #6086:
Hmm, any idea what could be happening with that test failure? Is there some special-case around sinking for
cmp
that needs to be extended totest
?
meithecatte commented on issue #6086:
Hmm, this optimization can turn one use at the IR level into two uses at the same instruction, at the opcode level. Perhaps this confuses the sinking logic?
cfallin commented on issue #6086:
Hmm, this optimization can turn one use at the IR level into two uses at the same instruction, at the opcode level. Perhaps this confuses the sinking logic?
Yes, we have some subtle invariants here where lowering
icmp
/fcmp
is special: we lower at every use (because otherwise we'd have to spill/restore flags), so we go to special lengths to not allow load sinking. See e.g. #3934. We'd like to come up with a more principled way to enforce this invariant, maybe a "special" lowering context state that knows we're lowering something that can be lowered multiple times; but right now we don't have that. Unfortunately I think this means that this change can't work as-is.
Last updated: Dec 23 2024 at 13:07 UTC