[SimplifyCFG][swifterror] Don't sink calls with swifterror params (#139015)
We've encountered an LLVM verification failure when building Swift with
the SimplifyCFG pass enabled. I found that
https://reviews.llvm.org/D158083 fixed this pass by preventing sinking
loads or stores of swifterror values, but it did not implement the same
protection for call or invokes.
In `Verifier.cpp`
[here](https://github.com/ellishg/llvm-project/blob/c68535581135a1513c9c4c1c7672307d4b5e616e/llvm/lib/IR/Verifier.cpp#L4360-L4364)
and
[here](https://github.com/ellishg/llvm-project/blob/c68535581135a1513c9c4c1c7672307d4b5e616e/llvm/lib/IR/Verifier.cpp#L3661-L3662)
we can see that swifterror values must also be used directly by call
instructions.
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 1db01b2..3dbd605 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -4220,12 +4220,19 @@
}
bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
+ const auto *Op = I->getOperand(OpIdx);
// We can't have a PHI with a metadata type.
- if (I->getOperand(OpIdx)->getType()->isMetadataTy())
+ if (Op->getType()->isMetadataTy())
+ return false;
+
+ // swifterror pointers can only be used by a load, store, or as a swifterror
+ // argument; swifterror pointers are not allowed to be used in select or phi
+ // instructions.
+ if (Op->isSwiftError())
return false;
// Early exit.
- if (!isa<Constant, InlineAsm>(I->getOperand(OpIdx)))
+ if (!isa<Constant, InlineAsm>(Op))
return true;
switch (I->getOpcode()) {
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 5cd244c..d94abea 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2208,14 +2208,6 @@
if (!I->isSameOperationAs(I0, Instruction::CompareUsingIntersectedAttrs))
return false;
- // swifterror pointers can only be used by a load or store; sinking a load
- // or store would require introducing a select for the pointer operand,
- // which isn't allowed for swifterror pointers.
- if (isa<StoreInst>(I) && I->getOperand(1)->isSwiftError())
- return false;
- if (isa<LoadInst>(I) && I->getOperand(0)->isSwiftError())
- return false;
-
// Treat MMRAs conservatively. This pass can be quite aggressive and
// could drop a lot of MMRAs otherwise.
if (MMRAMetadata(*I) != I0MMRA)
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-sink-swifterror-store.ll b/llvm/test/Transforms/SimplifyCFG/hoist-sink-swifterror-store.ll
index 0c13f57..5dff39c 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-sink-swifterror-store.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-sink-swifterror-store.ll
@@ -3,6 +3,8 @@
declare void @clobber1()
declare void @clobber2()
+declare swiftcc void @foo(ptr swifterror)
+declare swiftcc void @bar(ptr swifterror, ptr)
; Do not try to sink the stores to the exit block, as this requires introducing
; a select for the pointer operand. This is not allowed for swifterror pointers.
@@ -76,6 +78,22 @@
; introduces a select for the pointer operand. This is not allowed for
; swifterror pointers.
define swiftcc ptr @sink_load(ptr %arg, ptr swifterror %arg1, i1 %c) {
+; CHECK-LABEL: define swiftcc ptr @sink_load
+; CHECK-SAME: (ptr [[ARG:%.*]], ptr swifterror [[ARG1:%.*]], i1 [[C:%.*]]) {
+; CHECK-NEXT: bb:
+; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK: then:
+; CHECK-NEXT: call void @clobber1()
+; CHECK-NEXT: [[L1:%.*]] = load ptr, ptr [[ARG]], align 8
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: else:
+; CHECK-NEXT: call void @clobber2()
+; CHECK-NEXT: [[L2:%.*]] = load ptr, ptr [[ARG1]], align 8
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[L1]], [[THEN]] ], [ [[L2]], [[ELSE]] ]
+; CHECK-NEXT: ret ptr [[P]]
+;
bb:
br i1 %c, label %then, label %else
@@ -127,3 +145,77 @@
%p = phi ptr [ %l1, %then ], [ %l2, %else ]
ret ptr %p
}
+
+
+define swiftcc void @sink_call(i1 %c) {
+; CHECK-LABEL: define swiftcc void @sink_call
+; CHECK-SAME: (i1 [[C:%.*]]) {
+; CHECK-NEXT: [[TMP1:%.*]] = alloca swifterror ptr, align 8
+; CHECK-NEXT: [[TMP2:%.*]] = alloca swifterror ptr, align 8
+; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK: then:
+; CHECK-NEXT: call void @clobber1()
+; CHECK-NEXT: call swiftcc void @foo(ptr [[TMP2]])
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: else:
+; CHECK-NEXT: call void @clobber2()
+; CHECK-NEXT: call swiftcc void @foo(ptr [[TMP1]])
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ %2 = alloca swifterror ptr, align 8
+ %3 = alloca swifterror ptr, align 8
+ br i1 %c, label %then, label %else
+
+then:
+ call void @clobber1()
+ call swiftcc void @foo(ptr %3)
+ br label %exit
+
+else:
+ call void @clobber2()
+ call swiftcc void @foo(ptr %2)
+ br label %exit
+
+exit:
+ ret void
+}
+
+
+define swiftcc void @safe_sink_call(i1 %c) {
+; CHECK-LABEL: define swiftcc void @safe_sink_call
+; CHECK-SAME: (i1 [[C:%.*]]) {
+; CHECK-NEXT: [[ERR:%.*]] = alloca swifterror ptr, align 8
+; CHECK-NEXT: [[A:%.*]] = alloca ptr, align 8
+; CHECK-NEXT: [[B:%.*]] = alloca ptr, align 8
+; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK: then:
+; CHECK-NEXT: call void @clobber1()
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: else:
+; CHECK-NEXT: call void @clobber2()
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: [[B_SINK:%.*]] = phi ptr [ [[B]], [[ELSE]] ], [ [[A]], [[THEN]] ]
+; CHECK-NEXT: call swiftcc void @bar(ptr [[ERR]], ptr [[B_SINK]])
+; CHECK-NEXT: ret void
+;
+ %err = alloca swifterror ptr, align 8
+ %a = alloca ptr, align 8
+ %b = alloca ptr, align 8
+ br i1 %c, label %then, label %else
+
+then:
+ call void @clobber1()
+ call swiftcc void @bar(ptr %err, ptr %a)
+ br label %exit
+
+else:
+ call void @clobber2()
+ call swiftcc void @bar(ptr %err, ptr %b)
+ br label %exit
+
+exit:
+ ret void
+}
