Skip to content

Commit 78f0af5

Browse files
authored
[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.
1 parent 6b7b289 commit 78f0af5

File tree

3 files changed

+101
-10
lines changed

3 files changed

+101
-10
lines changed

llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4220,12 +4220,19 @@ void llvm::maybeMarkSanitizerLibraryCallNoBuiltin(
42204220
}
42214221

42224222
bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
4223+
const auto *Op = I->getOperand(OpIdx);
42234224
// We can't have a PHI with a metadata type.
4224-
if (I->getOperand(OpIdx)->getType()->isMetadataTy())
4225+
if (Op->getType()->isMetadataTy())
4226+
return false;
4227+
4228+
// swifterror pointers can only be used by a load, store, or as a swifterror
4229+
// argument; swifterror pointers are not allowed to be used in select or phi
4230+
// instructions.
4231+
if (Op->isSwiftError())
42254232
return false;
42264233

42274234
// Early exit.
4228-
if (!isa<Constant, InlineAsm>(I->getOperand(OpIdx)))
4235+
if (!isa<Constant, InlineAsm>(Op))
42294236
return true;
42304237

42314238
switch (I->getOpcode()) {

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2208,14 +2208,6 @@ static bool canSinkInstructions(
22082208
if (!I->isSameOperationAs(I0, Instruction::CompareUsingIntersectedAttrs))
22092209
return false;
22102210

2211-
// swifterror pointers can only be used by a load or store; sinking a load
2212-
// or store would require introducing a select for the pointer operand,
2213-
// which isn't allowed for swifterror pointers.
2214-
if (isa<StoreInst>(I) && I->getOperand(1)->isSwiftError())
2215-
return false;
2216-
if (isa<LoadInst>(I) && I->getOperand(0)->isSwiftError())
2217-
return false;
2218-
22192211
// Treat MMRAs conservatively. This pass can be quite aggressive and
22202212
// could drop a lot of MMRAs otherwise.
22212213
if (MMRAMetadata(*I) != I0MMRA)

llvm/test/Transforms/SimplifyCFG/hoist-sink-swifterror-store.ll

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
declare void @clobber1()
55
declare void @clobber2()
6+
declare swiftcc void @foo(ptr swifterror)
7+
declare swiftcc void @bar(ptr swifterror, ptr)
68

79
; Do not try to sink the stores to the exit block, as this requires introducing
810
; a select for the pointer operand. This is not allowed for swifterror pointers.
@@ -76,6 +78,22 @@ exit:
7678
; introduces a select for the pointer operand. This is not allowed for
7779
; swifterror pointers.
7880
define swiftcc ptr @sink_load(ptr %arg, ptr swifterror %arg1, i1 %c) {
81+
; CHECK-LABEL: define swiftcc ptr @sink_load
82+
; CHECK-SAME: (ptr [[ARG:%.*]], ptr swifterror [[ARG1:%.*]], i1 [[C:%.*]]) {
83+
; CHECK-NEXT: bb:
84+
; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[ELSE:%.*]]
85+
; CHECK: then:
86+
; CHECK-NEXT: call void @clobber1()
87+
; CHECK-NEXT: [[L1:%.*]] = load ptr, ptr [[ARG]], align 8
88+
; CHECK-NEXT: br label [[EXIT:%.*]]
89+
; CHECK: else:
90+
; CHECK-NEXT: call void @clobber2()
91+
; CHECK-NEXT: [[L2:%.*]] = load ptr, ptr [[ARG1]], align 8
92+
; CHECK-NEXT: br label [[EXIT]]
93+
; CHECK: exit:
94+
; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[L1]], [[THEN]] ], [ [[L2]], [[ELSE]] ]
95+
; CHECK-NEXT: ret ptr [[P]]
96+
;
7997
bb:
8098
br i1 %c, label %then, label %else
8199

@@ -127,3 +145,77 @@ exit:
127145
%p = phi ptr [ %l1, %then ], [ %l2, %else ]
128146
ret ptr %p
129147
}
148+
149+
150+
define swiftcc void @sink_call(i1 %c) {
151+
; CHECK-LABEL: define swiftcc void @sink_call
152+
; CHECK-SAME: (i1 [[C:%.*]]) {
153+
; CHECK-NEXT: [[TMP1:%.*]] = alloca swifterror ptr, align 8
154+
; CHECK-NEXT: [[TMP2:%.*]] = alloca swifterror ptr, align 8
155+
; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[ELSE:%.*]]
156+
; CHECK: then:
157+
; CHECK-NEXT: call void @clobber1()
158+
; CHECK-NEXT: call swiftcc void @foo(ptr [[TMP2]])
159+
; CHECK-NEXT: br label [[EXIT:%.*]]
160+
; CHECK: else:
161+
; CHECK-NEXT: call void @clobber2()
162+
; CHECK-NEXT: call swiftcc void @foo(ptr [[TMP1]])
163+
; CHECK-NEXT: br label [[EXIT]]
164+
; CHECK: exit:
165+
; CHECK-NEXT: ret void
166+
;
167+
%2 = alloca swifterror ptr, align 8
168+
%3 = alloca swifterror ptr, align 8
169+
br i1 %c, label %then, label %else
170+
171+
then:
172+
call void @clobber1()
173+
call swiftcc void @foo(ptr %3)
174+
br label %exit
175+
176+
else:
177+
call void @clobber2()
178+
call swiftcc void @foo(ptr %2)
179+
br label %exit
180+
181+
exit:
182+
ret void
183+
}
184+
185+
186+
define swiftcc void @safe_sink_call(i1 %c) {
187+
; CHECK-LABEL: define swiftcc void @safe_sink_call
188+
; CHECK-SAME: (i1 [[C:%.*]]) {
189+
; CHECK-NEXT: [[ERR:%.*]] = alloca swifterror ptr, align 8
190+
; CHECK-NEXT: [[A:%.*]] = alloca ptr, align 8
191+
; CHECK-NEXT: [[B:%.*]] = alloca ptr, align 8
192+
; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[ELSE:%.*]]
193+
; CHECK: then:
194+
; CHECK-NEXT: call void @clobber1()
195+
; CHECK-NEXT: br label [[EXIT:%.*]]
196+
; CHECK: else:
197+
; CHECK-NEXT: call void @clobber2()
198+
; CHECK-NEXT: br label [[EXIT]]
199+
; CHECK: exit:
200+
; CHECK-NEXT: [[B_SINK:%.*]] = phi ptr [ [[B]], [[ELSE]] ], [ [[A]], [[THEN]] ]
201+
; CHECK-NEXT: call swiftcc void @bar(ptr [[ERR]], ptr [[B_SINK]])
202+
; CHECK-NEXT: ret void
203+
;
204+
%err = alloca swifterror ptr, align 8
205+
%a = alloca ptr, align 8
206+
%b = alloca ptr, align 8
207+
br i1 %c, label %then, label %else
208+
209+
then:
210+
call void @clobber1()
211+
call swiftcc void @bar(ptr %err, ptr %a)
212+
br label %exit
213+
214+
else:
215+
call void @clobber2()
216+
call swiftcc void @bar(ptr %err, ptr %b)
217+
br label %exit
218+
219+
exit:
220+
ret void
221+
}

0 commit comments

Comments
 (0)