Skip to content

Commit e3e2849

Browse files
committed
Fix a SimplifyCFG typo that leads to unbounded optimization
Fixes rdar://73357726 ([SR-14068]: Compiling with optimisation runs indefinitely for grpc-swift) The root cause of this problem is that SimplifyCFG::tryJumpThreading jump threads into loops, effectively peeling loops. This is not the right way to implement loop peeling. That belongs in a loop optimization pass. There's is simply no sane way to control jump threading if it is allowed across loop boundaries, both from the standpoint of requiring optimizations to terminate and from the standpoint of reducing senseless code bloat. SimplifyCFG does have a mechanism to avoid jump-threading into loop in most cases. That mechanism would actually prevent the infinite loop peeling in this particular case if it were implemented correctly. But the original implementation circa 2014 appears to have a typo. This commit fixes that obvious bug. I do not think it's a sufficient to ensure we never see the bad behavior. I will file separate bugs for the broader issue. This bad behavior was exposed incidentally by splitting critical edges. Without edge splitting, SimplifyCFG::simplifyBlocks only performs "jump threading" once, creating a critical edge to the loop header. Because simplifyBlocks works under the assumption that there are no critical edges, it never attempts to perform jump threading again. In other words, the presence of the critical edge "breaks" the optimization, preventing it from continuing as intended. With edge splitting, the simplifyBlocks worklist performs "jump threading" followed by "jump to trampoline" removal, which creates a new loop-back edge to the original loop header. This is fine. However, simplifyBlocks iteratively attempts all optimizations up to a fix point and it does not stop at loop headers! So, splitting the critical edge causes simplifyBlocks to work as intended, which leads to infinite loop peeling. The end result is an infinite sequence of nested loops. Each peeled iteration is actually within the parent loop. (cherry picked from commit 8948f75)
1 parent 3693d92 commit e3e2849

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1401,7 +1401,7 @@ bool SimplifyCFG::simplifyBranchBlock(BranchInst *BI) {
14011401
// Eliminating the trampoline can expose opportunities to improve the
14021402
// new block we branch to.
14031403
if (LoopHeaders.count(DestBB))
1404-
LoopHeaders.insert(BB);
1404+
LoopHeaders.insert(trampolineDest.destBB);
14051405

14061406
addToWorklist(trampolineDest.destBB);
14071407
BI->eraseFromParent();

test/SILOptimizer/simplify_cfg_simple.sil

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ sil_stage canonical
77
import Builtin
88
import Swift
99

10+
internal enum Enum {
11+
case one
12+
case two
13+
}
14+
1015
// CHECK-LABEL: sil @simple_test : $@convention(thin) () -> () {
1116
// CHECK: bb0:
1217
// CHECK-NEXT: tuple
@@ -27,3 +32,64 @@ bb3:
2732
%9999 = tuple ()
2833
return %9999 : $()
2934
}
35+
36+
// Test that SimplifyCFG::simplifyBlocks, tryJumpThreading does not
37+
// perform unbounded loop peeling.
38+
//
39+
// rdar://73357726 ([SR-14068]: Compiling with optimisation runs indefinitely for grpc-swift)
40+
// CHECK-LABEL: sil @testInfinitePeeling : $@convention(method) (Builtin.Int64, Enum) -> () {
41+
//
42+
// There is only one switch_enum blocks, and it is no longer in a loop.
43+
// CHECK: bb0(%0 : $Builtin.Int64, %1 : $Enum):
44+
// CHECK: switch_enum %1 : $Enum, case #Enum.one!enumelt: bb3, case #Enum.two!enumelt: bb4
45+
// CHECK: bb1:
46+
// CHECK: br bb8
47+
// CHECK: bb2:
48+
// CHECK: br bb5(%{{.*}} : $Enum)
49+
//
50+
// This is the original cond_br block
51+
// CHECK: bb3:
52+
// CHECK: cond_br %{{.*}}, bb2, bb1
53+
// CHECK: bb4:
54+
// CHECK: br bb5(%1 : $Enum)
55+
//
56+
// This is the cond_br block after jump-threading.
57+
// CHECK: bb5(%{{.*}} : $Enum):
58+
// CHECK: cond_br %{{.*}}, bb6, bb7
59+
// CHECK: bb6:
60+
// CHECK: br bb5(%{{.*}} : $Enum)
61+
// CHECK: bb7:
62+
// CHECK: br bb8
63+
// CHECK: bb8:
64+
// CHECK: return %19 : $()
65+
// CHECK-LABEL: } // end sil function 'testInfinitePeeling'
66+
sil @testInfinitePeeling : $@convention(method) (Builtin.Int64, Enum) -> () {
67+
bb0(%0 : $Builtin.Int64, %1 : $Enum):
68+
%2 = integer_literal $Builtin.Int64, 99999999
69+
br bb1(%0 : $Builtin.Int64, %1 : $Enum)
70+
71+
bb1(%4 : $Builtin.Int64, %5 : $Enum):
72+
switch_enum %5 : $Enum, case #Enum.one!enumelt: bb4, default bb5
73+
74+
bb2(%7 : $Builtin.Int64, %8 : $Enum):
75+
%9 = builtin "cmp_slt_Int64"(%2 : $Builtin.Int64, %7 : $Builtin.Int64) : $Builtin.Int1
76+
cond_br %9, bb3, bb6
77+
78+
bb3:
79+
br bb1(%7 : $Builtin.Int64, %8 : $Enum)
80+
81+
bb4:
82+
%12 = integer_literal $Builtin.Int64, 1
83+
%13 = integer_literal $Builtin.Int1, -1
84+
%14 = builtin "sadd_with_overflow_Int64"(%4 : $Builtin.Int64, %12 : $Builtin.Int64, %13 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
85+
%15 = tuple_extract %14 : $(Builtin.Int64, Builtin.Int1), 0
86+
%16 = enum $Enum, #Enum.two!enumelt
87+
br bb2(%15 : $Builtin.Int64, %16 : $Enum)
88+
89+
bb5:
90+
br bb2(%2 : $Builtin.Int64, %5 : $Enum)
91+
92+
bb6:
93+
%19 = tuple ()
94+
return %19 : $()
95+
}

0 commit comments

Comments
 (0)