This is done by allowing speculation of udiv if we can prove the
denominator is non-zero.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/CodeGen/SelectionDAG.h | ||
---|---|---|
2345 | Why don't refactor it directly? |
llvm/include/llvm/CodeGen/SelectionDAG.h | ||
---|---|---|
2345 | I figured they have seperate purposes. The Opcode version is checking whether an opcode is unconditionally safe to speculate, node is asking about this specific case. But I'll check the other usages and see if the refactored version makes sense. |
Does that increase the review burden? Now we have to audit the other transforms to make sure the don't change operand 1 in a way that would be unsafe?
Plus we should have additional tests?
Sure. I'll leave as previous version in this patch and make the refactor a follow up.
Edit: Looking at some of the other locations more closely. I think it makes
sense to leave as a seperate API. isSafeToSpeculativelyExecute is used to query
about the opcode without knowing the operands so serves a different purpose.
It wouldn't be NFC. It would make some UDIV instructions supported by additional transforms.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
2438 | but UREM is not handled above.. |
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | ||
---|---|---|
2438 | Well the comment still holds true, but indeed could include urem in this patch. Will update. |
Why don't refactor it directly?