Skip to content

Commit 6924bad

Browse files
committed
SIL: fix a bug which can cause the stack nesting to get broken by loop unrolling
We must not unroll a loop with alloc_ref [stack] if a dealloc_stack is outside the loop. related to rdar://problem/28878079
1 parent dfe2f43 commit 6924bad

File tree

2 files changed

+76
-6
lines changed

2 files changed

+76
-6
lines changed

lib/SIL/LoopInfo.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ SILLoopInfo::SILLoopInfo(SILFunction *F, DominanceInfo *DT) {
3636
}
3737

3838
bool SILLoop::canDuplicate(SILInstruction *I) const {
39-
// The dealloc_stack of an alloc_stack must be in the loop, otherwise the
40-
// dealloc_stack will be fed by a phi node of two alloc_stacks.
41-
if (auto *Alloc = dyn_cast<AllocStackInst>(I)) {
42-
for (auto *UI : Alloc->getUses()) {
43-
if (auto *Dealloc = dyn_cast<DeallocStackInst>(UI->getUser())) {
44-
if (!contains(Dealloc->getParent()))
39+
// The deallocation of a stack allocation must be in the loop, otherwise the
40+
// deallocation will be fed by a phi node of two allocations.
41+
if (I->isAllocatingStack()) {
42+
for (auto *UI : I->getUses()) {
43+
if (UI->getUser()->isDeallocatingStack()) {
44+
if (!contains(UI->getUser()->getParent()))
4545
return false;
4646
}
4747
}
@@ -76,6 +76,8 @@ bool SILLoop::canDuplicate(SILInstruction *I) const {
7676
return false;
7777
}
7878

79+
assert(I->isTriviallyDuplicatable() &&
80+
"Code here must match isTriviallyDuplicatable in SILInstruction");
7981
return true;
8082
}
8183

test/SILOptimizer/loop_unroll.sil

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,71 @@ bb3:
7070
%8 = tuple()
7171
return %8 : $()
7272
}
73+
74+
class B {}
75+
76+
// CHECK-LABEL: sil @unroll_with_stack_allocation
77+
// CHECK: = alloc_ref [stack]
78+
// CHECK: dealloc_ref [stack]
79+
// CHECK: = alloc_ref [stack]
80+
// CHECK: dealloc_ref [stack]
81+
// CHECK: }
82+
sil @unroll_with_stack_allocation : $@convention(thin) () -> () {
83+
bb0:
84+
%0 = integer_literal $Builtin.Int64, 0
85+
%1 = integer_literal $Builtin.Int64, 1
86+
%2 = integer_literal $Builtin.Int64, 2
87+
%3 = integer_literal $Builtin.Int1, 1
88+
br bb1(%0 : $Builtin.Int64)
89+
90+
bb1(%4 : $Builtin.Int64):
91+
%a = alloc_ref [stack] $B
92+
br bb2
93+
94+
bb2:
95+
%5 = builtin "sadd_with_overflow_Int64"(%4 : $Builtin.Int64, %1 : $Builtin.Int64, %3 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
96+
%6 = tuple_extract %5 : $(Builtin.Int64, Builtin.Int1), 0
97+
%7 = builtin "cmp_eq_Int64"(%6 : $Builtin.Int64, %2 : $Builtin.Int64) : $Builtin.Int1
98+
dealloc_ref [stack] %a : $B
99+
cond_br %7, bb4, bb3
100+
101+
bb3:
102+
br bb1(%6 : $Builtin.Int64)
103+
104+
bb4:
105+
%8 = tuple()
106+
return %8 : $()
107+
}
108+
109+
// CHECK-LABEL: sil @dont_copy_stack_allocation_with_dealloc_outside_loop
110+
// CHECK: = alloc_ref [stack]
111+
// CHECK: bb2:
112+
// CHECK: dealloc_ref [stack]
113+
// CHECK: bb3:
114+
// CHECK: dealloc_ref [stack]
115+
// CHECK-NEXT: tuple
116+
// CHECK-NEXT: return
117+
sil @dont_copy_stack_allocation_with_dealloc_outside_loop : $@convention(thin) () -> () {
118+
bb0:
119+
%0 = integer_literal $Builtin.Int64, 0
120+
%1 = integer_literal $Builtin.Int64, 1
121+
%2 = integer_literal $Builtin.Int64, 2
122+
%3 = integer_literal $Builtin.Int1, 1
123+
br bb1(%0 : $Builtin.Int64)
124+
125+
bb1(%4 : $Builtin.Int64):
126+
%a = alloc_ref [stack] $B
127+
%5 = builtin "sadd_with_overflow_Int64"(%4 : $Builtin.Int64, %1 : $Builtin.Int64, %3 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
128+
%6 = tuple_extract %5 : $(Builtin.Int64, Builtin.Int1), 0
129+
%7 = builtin "cmp_eq_Int64"(%6 : $Builtin.Int64, %2 : $Builtin.Int64) : $Builtin.Int1
130+
cond_br %7, bb3, bb2
131+
132+
bb2:
133+
dealloc_ref [stack] %a : $B
134+
br bb1(%6 : $Builtin.Int64)
135+
136+
bb3:
137+
dealloc_ref [stack] %a : $B
138+
%8 = tuple()
139+
return %8 : $()
140+
}

0 commit comments

Comments
 (0)