aboutsummaryrefslogtreecommitdiff
path: root/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
diff options
context:
space:
mode:
authorHeejin Ahn <aheejin@gmail.com>2024-10-09 14:31:16 -0700
committerGitHub <noreply@github.com>2024-10-09 14:31:16 -0700
commit115cb402d8ed91f94d22afcc4c2c9ed9def53cc7 (patch)
treef115349986cb72c1bb46598523d3db807bfc0ffa /llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
parent853c43d04a378c379e49db552e856f02a5ad9216 (diff)
downloadllvm-115cb402d8ed91f94d22afcc4c2c9ed9def53cc7.zip
llvm-115cb402d8ed91f94d22afcc4c2c9ed9def53cc7.tar.gz
llvm-115cb402d8ed91f94d22afcc4c2c9ed9def53cc7.tar.bz2
[WebAssembly] Don't fold non-nuw add/sub in FastISel (#111278)
We should not fold one of add/sub operands into a load/store's offset when `nuw` (no unsigned wrap) is not present, because the address calculation, which adds the offset with the operand, does not wrap. This is handled correctly in the normal ISel: https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp#L328-L332 but not in FastISel. This positivity check in FastISel is not sufficient to avoid this case fully: https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp#L348-L352 because 1. Even if RHS is within signed int range, depending on the value of the LHS, the resulting value can exceed uint32 max. 2. When one of the operands is a label, `Address` can contain a `GlobalValue` and a `Reg` at the same time, so the `GlobalValue` becomes incorrectly an offset: https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp#L53-L69 https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp#L409-L417 Both cases are in the newly added test. We should handle `SUB` too because `SUB` is the same as `ADD` when RHS's sign changes. I checked why our current normal ISel only handles `ADD`, and the reason it's OK for the normal ISel to handle only `ADD` seems that DAGCombiner replaces `SUB` with `ADD` here: https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L3904-L3907 Fixes #111018.
Diffstat (limited to 'llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp')
-rw-r--r--llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp12
1 files changed, 12 insertions, 0 deletions
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index 317c646..7c90fff 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -337,6 +337,12 @@ bool WebAssemblyFastISel::computeAddress(const Value *Obj, Address &Addr) {
break;
}
case Instruction::Add: {
+ // We should not fold operands into an offset when 'nuw' (no unsigned wrap)
+ // is not present, because the address calculation does not wrap.
+ if (auto *OFBinOp = dyn_cast<OverflowingBinaryOperator>(U))
+ if (!OFBinOp->hasNoUnsignedWrap())
+ break;
+
// Adds of constants are common and easy enough.
const Value *LHS = U->getOperand(0);
const Value *RHS = U->getOperand(1);
@@ -360,6 +366,12 @@ bool WebAssemblyFastISel::computeAddress(const Value *Obj, Address &Addr) {
break;
}
case Instruction::Sub: {
+ // We should not fold operands into an offset when 'nuw' (no unsigned wrap)
+ // is not present, because the address calculation does not wrap.
+ if (auto *OFBinOp = dyn_cast<OverflowingBinaryOperator>(U))
+ if (!OFBinOp->hasNoUnsignedWrap())
+ break;
+
// Subs of constants are common and easy enough.
const Value *LHS = U->getOperand(0);
const Value *RHS = U->getOperand(1);