[llvm] 115cb40 - [WebAssembly] Don't fold non-nuw add/sub in FastISel (#111278)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 9 14:31:19 PDT 2024
Author: Heejin Ahn
Date: 2024-10-09T14:31:16-07:00
New Revision: 115cb402d8ed91f94d22afcc4c2c9ed9def53cc7
URL: https://github.com/llvm/llvm-project/commit/115cb402d8ed91f94d22afcc4c2c9ed9def53cc7
DIFF: https://github.com/llvm/llvm-project/commit/115cb402d8ed91f94d22afcc4c2c9ed9def53cc7.diff
LOG: [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.
Added:
llvm/test/CodeGen/WebAssembly/fast-isel-no-offset.ll
Modified:
llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index 317c6463985dcb..7c90fff2a5c1d7 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);
diff --git a/llvm/test/CodeGen/WebAssembly/fast-isel-no-offset.ll b/llvm/test/CodeGen/WebAssembly/fast-isel-no-offset.ll
new file mode 100644
index 00000000000000..d4ba1f3bc4a450
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/fast-isel-no-offset.ll
@@ -0,0 +1,106 @@
+; RUN: llc < %s -asm-verbose=false -fast-isel -fast-isel-abort=1 -verify-machineinstrs | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+; FastISel should not fold one of the add/sub operands into a load/store's
+; offset when 'nuw' (no unsigned wrap) is not present, because the address
+; calculation does not wrap. When there is an add/sub and nuw is not present, we
+; bail out of FastISel.
+
+ at mylabel = external global ptr
+
+; CHECK-LABEL: dont_fold_non_nuw_add_load:
+; CHECK: local.get 0
+; CHECK-NEXT: i32.const 2147483644
+; CHECK-NEXT: i32.add
+; CHECK-NEXT: i32.load 0
+define i32 @dont_fold_non_nuw_add_load(ptr %p) {
+ %q = ptrtoint ptr %p to i32
+ %r = add i32 %q, 2147483644
+ %s = inttoptr i32 %r to ptr
+ %t = load i32, ptr %s
+ ret i32 %t
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_add_store:
+; CHECK: local.get 0
+; CHECK-NEXT: i32.const 2147483644
+; CHECK-NEXT: i32.add
+; CHECK-NEXT: i32.const 5
+; CHECK-NEXT: i32.store 0
+define void @dont_fold_non_nuw_add_store(ptr %p) {
+ %q = ptrtoint ptr %p to i32
+ %r = add i32 %q, 2147483644
+ %s = inttoptr i32 %r to ptr
+ store i32 5, ptr %s
+ ret void
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_add_load_2:
+; CHECK: i32.const mylabel
+; CHECK-NEXT: i32.const -4
+; CHECK-NEXT: i32.add
+; CHECK-NEXT: i32.load 0
+define i32 @dont_fold_non_nuw_add_load_2() {
+ %t = load i32, ptr inttoptr (i32 add (i32 ptrtoint (ptr @mylabel to i32), i32 -4) to ptr), align 4
+ ret i32 %t
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_add_store_2:
+; CHECK: i32.const mylabel
+; CHECK-NEXT: i32.const -4
+; CHECK-NEXT: i32.add
+; CHECK-NEXT: i32.const 5
+; CHECK-NEXT: i32.store 0
+define void @dont_fold_non_nuw_add_store_2() {
+ store i32 5, ptr inttoptr (i32 add (i32 ptrtoint (ptr @mylabel to i32), i32 -4) to ptr), align 4
+ ret void
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_sub_load:
+; CHECK: local.get 0
+; CHECK-NEXT: i32.const -2147483644
+; CHECK-NEXT: i32.sub
+; CHECK-NEXT: i32.load 0
+define i32 @dont_fold_non_nuw_sub_load(ptr %p) {
+ %q = ptrtoint ptr %p to i32
+ %r = sub i32 %q, -2147483644
+ %s = inttoptr i32 %r to ptr
+ %t = load i32, ptr %s
+ ret i32 %t
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_sub_store:
+; CHECK: local.get 0
+; CHECK-NEXT: i32.const -2147483644
+; CHECK-NEXT: i32.sub
+; CHECK-NEXT: i32.const 5
+; CHECK-NEXT: i32.store 0
+define void @dont_fold_non_nuw_sub_store(ptr %p) {
+ %q = ptrtoint ptr %p to i32
+ %r = sub i32 %q, -2147483644
+ %s = inttoptr i32 %r to ptr
+ store i32 5, ptr %s
+ ret void
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_sub_load_2:
+; CHECK: i32.const mylabel
+; CHECK-NEXT: i32.const 4
+; CHECK-NEXT: i32.sub
+; CHECK-NEXT: i32.load 0
+define i32 @dont_fold_non_nuw_sub_load_2() {
+ %t = load i32, ptr inttoptr (i32 sub (i32 ptrtoint (ptr @mylabel to i32), i32 4) to ptr), align 4
+ ret i32 %t
+}
+
+; CHECK-LABEL: dont_fold_non_nuw_sub_store_2:
+; CHECK: i32.const mylabel
+; CHECK-NEXT: i32.const 4
+; CHECK-NEXT: i32.sub
+; CHECK-NEXT: i32.const 5
+; CHECK-NEXT: i32.store 0
+define void @dont_fold_non_nuw_sub_store_2() {
+ store i32 5, ptr inttoptr (i32 sub (i32 ptrtoint (ptr @mylabel to i32), i32 4) to ptr), align 4
+ ret void
+}
diff --git a/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll b/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll
index 6a1304cb9a93a8..75cb5b66b3ebee 100644
--- a/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll
+++ b/llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll
@@ -14,7 +14,7 @@ target triple = "wasm32-unknown-unknown"
define i32 @foo() {
%stack_addr = alloca i32
%stack_i = ptrtoint ptr %stack_addr to i32
- %added = add i32 %stack_i, undef
+ %added = add nuw i32 %stack_i, undef
%added_addr = inttoptr i32 %added to ptr
%ret = load i32, ptr %added_addr
ret i32 %ret
More information about the llvm-commits
mailing list