Skip to content

Commit 742328d

Browse files
adrian-prantladrian-prantl
committed
SILInliner: Fix scope & source location of terminator instructions.
Instead of cloning existing instructions (and correctly remapping their scopes) SILCloner just creates a fresh instruction for the terminator and forgets to set the scope accordingly. This bug flew under the radar because the SIL verifier for continiuous scopes doesn't look at terminator instructions. rdar://97617367
1 parent e30181d commit 742328d

File tree

2 files changed

+93
-12
lines changed

2 files changed

+93
-12
lines changed

‎lib/SILOptimizer/Utils/SILInliner.cpp

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -518,50 +518,56 @@ void SILInlineCloner::cloneInline(ArrayRef<SILValue> AppliedArgs) {
518518
}
519519

520520
void SILInlineCloner::visitTerminator(SILBasicBlock *BB) {
521+
TermInst *Terminator = BB->getTerminator();
521522
// Coroutine terminators need special handling.
522523
if (BeginApply) {
523524
if (BeginApply->processTerminator(
524-
BB->getTerminator(), ReturnToBB,
525+
Terminator, ReturnToBB,
525526
[=](SILBasicBlock *Block) -> SILBasicBlock * {
526527
return this->remapBasicBlock(Block);
527528
},
528-
[=](SILValue Val) -> SILValue { return this->getMappedValue(Val); }))
529+
[=](SILValue Val) -> SILValue {
530+
return this->getMappedValue(Val);
531+
}))
529532
return;
530533
}
531534

532-
// Modify return terminators to branch to the return-to BB, rather than
533-
// trying to clone the ReturnInst.
534-
if (auto *RI = dyn_cast<ReturnInst>(BB->getTerminator())) {
535+
// Modify return terminators to branch to the return-to BB, rather
536+
// than trying to clone the ReturnInst. Because of that, the scope
537+
// needs to be remapped manually.
538+
getBuilder().setCurrentDebugScope(getOpScope(Terminator->getDebugScope()));
539+
if (auto *RI = dyn_cast<ReturnInst>(Terminator)) {
535540
auto returnedValue = getMappedValue(RI->getOperand());
536-
getBuilder().createBranch(Loc.getValue(), ReturnToBB, returnedValue);
541+
getBuilder().createBranch(getOpLocation(RI->getLoc()), ReturnToBB,
542+
returnedValue);
537543
return;
538544
}
539545

540546
// Modify throw terminators to branch to the error-return BB, rather than
541547
// trying to clone the ThrowInst.
542-
if (auto *TI = dyn_cast<ThrowInst>(BB->getTerminator())) {
548+
if (auto *TI = dyn_cast<ThrowInst>(Terminator)) {
549+
SILLocation Loc = getOpLocation(TI->getLoc());
543550
switch (Apply.getKind()) {
544551
case FullApplySiteKind::ApplyInst:
545552
assert(cast<ApplyInst>(Apply)->isNonThrowing()
546553
&& "apply of a function with error result must be non-throwing");
547-
getBuilder().createUnreachable(Loc.getValue());
554+
getBuilder().createUnreachable(Loc);
548555
return;
549556
case FullApplySiteKind::BeginApplyInst:
550557
assert(cast<BeginApplyInst>(Apply)->isNonThrowing()
551558
&& "apply of a function with error result must be non-throwing");
552-
getBuilder().createUnreachable(Loc.getValue());
559+
getBuilder().createUnreachable(Loc);
553560
return;
554561
case FullApplySiteKind::TryApplyInst:
555562
auto tryAI = cast<TryApplyInst>(Apply);
556563
auto returnedValue = getMappedValue(TI->getOperand());
557-
getBuilder().createBranch(Loc.getValue(), tryAI->getErrorBB(),
558-
returnedValue);
564+
getBuilder().createBranch(Loc, tryAI->getErrorBB(), returnedValue);
559565
return;
560566
}
561567
}
562568
// Otherwise use normal visitor, which clones the existing instruction
563569
// but remaps basic blocks and values.
564-
visit(BB->getTerminator());
570+
visit(Terminator);
565571
}
566572

567573
void SILInlineCloner::preFixUp(SILFunction *calleeFunction) {

‎test/SILOptimizer/inline_terminator_scopes.sil

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-print-debuginfo | %FileCheck %s
2+
3+
// Generated from:
4+
// func g(_ x: Int) -> Int {
5+
// if (x > 0) {
6+
// return 1
7+
// }
8+
// return 0
9+
// }
10+
//
11+
// public func f(_ x: Int) -> Int { return g(x) }
12+
13+
// CHECK: sil @$s1a1fyS2iF : $@convention(thin) (Int) -> Int {
14+
// CHECK: bb0(%0 : $Int):
15+
// CHECK-NEXT: scope [[SCOPE_10:[0-9]+]]
16+
// CHECK: cond_br {{.*}}, bb1, bb2, loc "t.swift":3:9, scope [[SCOPE_10]]
17+
// CHECK: bb1:
18+
// CHECK-NEXT: scope [[SCOPE_11:[0-9]+]]
19+
// CHECK: br bb3({{.*}} : $Int), loc "t.swift":4:5, scope [[SCOPE_11]]
20+
// CHECK: bb2:
21+
// CHECK-NEXT: scope [[SCOPE_9:[0-9]+]]
22+
// CHECK: br bb3({{.*}} : $Int), loc "t.swift":6:3, scope [[SCOPE_9]]
23+
// CHECK: bb3({{.*}} : $Int):
24+
25+
sil_stage canonical
26+
27+
import Builtin
28+
import Swift
29+
import SwiftShims
30+
31+
func g(_ x: Int) -> Int
32+
33+
public func f(_ x: Int) -> Int
34+
35+
sil_scope 1 { loc "t.swift":2:6 parent @$s1a1gyS2iF : $@convention(thin) (Int) -> Int }
36+
sil_scope 2 { loc "t.swift":2:25 parent 1 }
37+
sil_scope 3 { loc "t.swift":3:3 parent 2 }
38+
sil_scope 4 { loc "t.swift":3:14 parent 3 }
39+
40+
// g(_:)
41+
sil hidden @$s1a1gyS2iF : $@convention(thin) (Int) -> Int {
42+
// %0 "x" // users: %3, %1
43+
bb0(%0 : $Int):
44+
%2 = integer_literal $Builtin.Int64, 0, loc "t.swift":3:11, scope 3 // user: %4
45+
%3 = struct_extract %0 : $Int, #Int._value, loc "t.swift":3:9, scope 3 // user: %4
46+
%4 = builtin "cmp_slt_Int64"(%2 : $Builtin.Int64, %3 : $Builtin.Int64) : $Builtin.Int1, loc "t.swift":3:9, scope 3 // user: %5
47+
cond_br %4, bb1, bb2, loc "t.swift":3:9, scope 3 // id: %5
48+
49+
bb1: // Preds: bb0
50+
%6 = integer_literal $Builtin.Int64, 1, loc "t.swift":4:12, scope 4 // user: %7
51+
%7 = struct $Int (%6 : $Builtin.Int64), loc "t.swift":4:12, scope 4 // user: %8
52+
br bb3(%7 : $Int), loc "t.swift":4:5, scope 4 // id: %8
53+
54+
bb2: // Preds: bb0
55+
%9 = integer_literal $Builtin.Int64, 0, loc "t.swift":6:10, scope 2 // user: %10
56+
%10 = struct $Int (%9 : $Builtin.Int64), loc "t.swift":6:10, scope 2 // user: %11
57+
br bb3(%10 : $Int), loc "t.swift":6:3, scope 2 // id: %11
58+
59+
// %12 // user: %13
60+
bb3(%12 : $Int): // Preds: bb2 bb1
61+
return %12 : $Int, loc "t.swift":7:1, scope 2 // id: %13
62+
} // end sil function '$s1a1gyS2iF'
63+
64+
sil_scope 7 { loc "t.swift":9:13 parent @$s1a1fyS2iF : $@convention(thin) (Int) -> Int }
65+
sil_scope 8 { loc "t.swift":9:32 parent 7 }
66+
67+
// f(_:)
68+
sil @$s1a1fyS2iF : $@convention(thin) (Int) -> Int {
69+
// %0 "x" // users: %3, %1
70+
bb0(%0 : $Int):
71+
// function_ref g(_:)
72+
%2 = function_ref @$s1a1gyS2iF : $@convention(thin) (Int) -> Int, loc "t.swift":9:41, scope 8 // user: %3
73+
%3 = apply %2(%0) : $@convention(thin) (Int) -> Int, loc "t.swift":9:41, scope 8 // user: %4
74+
return %3 : $Int, loc "t.swift":9:34, scope 8 // id: %4
75+
} // end sil function '$s1a1fyS2iF'

0 commit comments

Comments
 (0)