Skip to content

Commit 5910829

Browse files
committed
[constant-prop] When replacing uses of destructure_tuple, only RAUW if we actually have uses since we may not delete the actual destructure.
The specific case where this happened here is: ``` // Worklist = [ %0 = struct $Int (...) %1 = $Klass %2 = tuple (%0, %1) (%3, %4) = destructure_tuple %2 store %3 to [trivial] %3stack store %4 to [init] %4stack ``` What would happen is we would visit the destructure_tuple, replace %3 with %0 but fail to propagate %4: ``` %0 = struct $Int (...) %1 = $Klass %2 = tuple (%0, %1) (%3, %4) = destructure_tuple %2 store %0 to [trivial] %3stack store %4 to [init] %4stack ``` This then causes the tuple to be added to the worklist. When we visit the tuple, we see that we have a destructure_tuple that is a user of the tuple, we see that it still has that Struct as a user despite us having constant propagated that component of the tuple. This then causes us to add the struct back to the worklist despite that tuple component having no uses. Then when we visit the struct. Which causes us to visit the tuple, etc. rdar://49947112
1 parent 1e032ad commit 5910829

File tree

2 files changed

+90
-2
lines changed

2 files changed

+90
-2
lines changed

lib/SILOptimizer/Utils/ConstantFolding.cpp

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ APInt swift::constantFoldCast(APInt val, const BuiltinInfo &BI) {
113113
SrcTy->castTo<BuiltinIntegerType>()->getGreatestWidth();
114114
uint32_t DestBitWidth =
115115
DestTy->castTo<BuiltinIntegerType>()->getGreatestWidth();
116-
116+
117117
APInt CastResV;
118118
if (SrcBitWidth == DestBitWidth) {
119119
return val;
@@ -1761,7 +1761,46 @@ ConstantFolder::processWorkList() {
17611761
FoldedUsers.insert(TI);
17621762
}
17631763

1764-
// We were able to fold, so all users should use the new folded value.
1764+
// We were able to fold, so all users should use the new folded
1765+
// value. If we don't have any such users, continue.
1766+
//
1767+
// NOTE: The reason why we check if our result has uses is that if
1768+
// User is a MultipleValueInstruction an infinite loop can result if
1769+
// User has a result different than the one at Index that we can not
1770+
// constant fold and if C's defining instruction is an aggregate that
1771+
// defines an operand of I.
1772+
//
1773+
// As an elucidating example, consider the following SIL:
1774+
//
1775+
// %w = integer_literal $Builtin.Word, 1
1776+
// %0 = struct $Int (%w : $Builtin.Word) (*)
1777+
// %1 = apply %f() : $@convention(thin) () -> @owned Klass
1778+
// %2 = tuple (%0 : $Int, %1 : $Klass)
1779+
// (%3, %4) = destructure_tuple %2 : $(Int, Klass)
1780+
// store %4 to [init] %mem2: %*Klass
1781+
//
1782+
// Without this check, we would infinite loop by processing our
1783+
// worklist as follows:
1784+
//
1785+
// 1. We visit %w and add %0 to the worklist unconditionally since it
1786+
// is a StructInst.
1787+
//
1788+
// 2. We visit %0 and then since %2 is a tuple, we add %2 to the
1789+
// worklist unconditionally.
1790+
//
1791+
// 3. We visit %2 and see that it has a destructure_tuple user. We see
1792+
// that we can simplify %3 -> %0, but cannot simplify %4. This
1793+
// means that if we just assume success if we can RAUW %3 without
1794+
// checking if we will actually replace anything, we will add %0's
1795+
// defining instruction (*) to the worklist. Q.E.D.
1796+
//
1797+
// In contrast, if we realize that RAUWing %3 does nothing and skip
1798+
// it, we exit the worklist as expected.
1799+
SILValue r = User->getResult(Index);
1800+
if (r->use_empty())
1801+
continue;
1802+
1803+
// Otherwise, do the RAUW.
17651804
User->getResult(Index)->replaceAllUsesWith(C);
17661805

17671806
// The new constant could be further folded now, add it to the

test/SILOptimizer/constant_propagation_ownership.sil

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
import Swift
55
import Builtin
66

7+
//////////////////
8+
// Declarations //
9+
//////////////////
10+
711
struct UInt {
812
var value: Builtin.Word
913
}
@@ -20,6 +24,15 @@ struct UInt64 {
2024
var value: Builtin.Int64
2125
}
2226

27+
class Klass {}
28+
29+
sil @klass_allocator : $@convention(method) (@thick Klass.Type) -> @owned Klass
30+
sil @generic_user : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
31+
32+
///////////
33+
// Tests //
34+
///////////
35+
2336
sil [ossa] @count_leading_zeros_corner_case : $@convention(thin) () -> Builtin.Int64 {
2437
bb0:
2538
%zero64 = integer_literal $Builtin.Int64, 0
@@ -1132,3 +1145,39 @@ bb2(%8 : @owned $AnSubObject):
11321145
bb3(%11 : $Builtin.Int1):
11331146
return %11 : $Builtin.Int1
11341147
}
1148+
1149+
sil [ossa] @do_not_infinite_loop : $@convention(thin) () -> () {
1150+
bb0:
1151+
%0 = integer_literal $Builtin.Word, 1
1152+
%1 = struct $Int (%0 : $Builtin.Word)
1153+
%2 = metatype $@thick Klass.Type
1154+
%3 = function_ref @klass_allocator : $@convention(method) (@thick Klass.Type) -> @owned Klass
1155+
%4 = apply %3(%2) : $@convention(method) (@thick Klass.Type) -> @owned Klass
1156+
// We can never in constant propagation forward %5 today since we would need
1157+
// to know that we are eliminting all of its uses. But the pass doesn't
1158+
// understand that today and so destroy_value %5 in bb2 stops our ability to
1159+
// optimize.
1160+
%5 = tuple (%1 : $Int, %4 : $Klass)
1161+
cond_br undef, bb1, bb2
1162+
1163+
bb1:
1164+
%6 = alloc_stack $(Int, Klass)
1165+
%7 = tuple_element_addr %6 : $*(Int, Klass), 0
1166+
%8 = tuple_element_addr %6 : $*(Int, Klass), 1
1167+
(%9, %10) = destructure_tuple %5 : $(Int, Klass)
1168+
store %1 to [trivial] %7 : $*Int
1169+
store %10 to [init] %8 : $*Klass
1170+
%13 = function_ref @generic_user : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
1171+
%14 = apply %13<(Int, Klass)>(%6) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
1172+
destroy_addr %6 : $*(Int, Klass)
1173+
dealloc_stack %6 : $*(Int, Klass)
1174+
br bb3
1175+
1176+
bb2:
1177+
destroy_value %5 : $(Int, Klass)
1178+
br bb3
1179+
1180+
bb3:
1181+
%9999 = tuple()
1182+
return %9999 : $()
1183+
}

0 commit comments

Comments
 (0)