Skip to content

Commit b3a3220

Browse files
atrickatrick
authored
Merge pull request #19020 from atrick/fix-critical-edges
Fix critical edges
2 parents 965a2d1 + 763ae67 commit b3a3220

File tree

15 files changed

+1089
-815
lines changed

15 files changed

+1089
-815
lines changed

‎lib/SIL/SILVerifier.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4594,29 +4594,26 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
45944594
return true;
45954595
};
45964596

4597-
SILModule &M = F->getModule();
45984597
for (auto &BB : *F) {
45994598
TermInst *TI = BB.getTerminator();
46004599
CurInstruction = TI;
46014600

4602-
// Check for non-cond_br critical edges in canonical SIL.
4603-
if (!isa<CondBranchInst>(TI) && M.getStage() == SILStage::Canonical) {
4601+
// Check for non-cond_br critical edges.
4602+
auto *CBI = dyn_cast<CondBranchInst>(TI);
4603+
if (!CBI) {
46044604
for (unsigned Idx = 0, e = BB.getSuccessors().size(); Idx != e; ++Idx) {
46054605
require(!isCriticalEdgePred(TI, Idx),
46064606
"non cond_br critical edges not allowed");
46074607
}
46084608
continue;
46094609
}
4610-
46114610
// In ownership qualified SIL, ban critical edges from CondBranchInst that
46124611
// have non-trivial arguments.
4612+
//
4613+
// FIXME: it would be far simpler to ban all critical edges in general.
46134614
if (!F->hasQualifiedOwnership())
46144615
continue;
46154616

4616-
auto *CBI = dyn_cast<CondBranchInst>(TI);
4617-
if (!CBI)
4618-
continue;
4619-
46204617
if (isCriticalEdgePred(CBI, CondBranchInst::TrueIdx)) {
46214618
require(
46224619
llvm::all_of(CBI->getTrueArgs(),

‎lib/SILOptimizer/Transforms/ARCCodeMotion.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,6 @@ STATISTIC(NumReleasesHoisted, "Number of releases hoisted");
9696

9797
llvm::cl::opt<bool> DisableARCCodeMotion("disable-arc-cm", llvm::cl::init(false));
9898

99-
/// Disable optimization if we have to break critical edges in the function.
100-
llvm::cl::opt<bool>
101-
DisableIfWithCriticalEdge("disable-with-critical-edge", llvm::cl::init(false));
102-
10399
//===----------------------------------------------------------------------===//
104100
// Block State
105101
//===----------------------------------------------------------------------===//
@@ -1156,11 +1152,6 @@ class ARCCodeMotion : public SILFunctionTransform {
11561152
if (!F->shouldOptimize())
11571153
return;
11581154

1159-
// Return if there is critical edge and we are disabling critical edge
1160-
// splitting.
1161-
if (DisableIfWithCriticalEdge && hasCriticalEdges(*F, false))
1162-
return;
1163-
11641155
LLVM_DEBUG(llvm::dbgs() << "*** ARCCM on function: " << F->getName()
11651156
<< " ***\n");
11661157

‎test/DebugInfo/return.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@ public func ifelseexpr() -> Int64 {
2020
} else {
2121
x.x -= 1
2222
}
23-
// CHECK: [[X:%.*]] = load %T6return1XC*, %T6return1XC** [[ALLOCA]]
24-
// CHECK: @swift_release to void (%T6return1XC*)*)(%T6return1XC* [[X]])
23+
// CHECK: [[L:%.*]] = load %T6return1XC*, %T6return1XC** [[ALLOCA]]
24+
// CHECK: @swift_release to void (%T6return1XC*)*)(%T6return1XC* [[L]])
2525
// CHECK-SAME: , !dbg ![[RELEASE:.*]]
26-
2726
// The ret instruction should be in the same scope as the return expression.
2827
// CHECK: ret{{.*}}, !dbg ![[RELEASE]]
2928
return x.x // CHECK: ![[RELEASE]] = !DILocation(line: [[@LINE]], column: 3

‎test/IRGen/dynamic_cast.sil

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ bb0(%0 : $*P):
6767
%1 = alloc_stack $S
6868
checked_cast_addr_br take_always P in %0 : $*P to S in %1 : $*S, bb1, bb2
6969
bb1:
70-
br bb2
70+
br bb3
7171
bb2:
72+
br bb3
73+
bb3:
7274
destroy_addr %1 : $*S
7375
dealloc_stack %1 : $*S
7476
%2 = tuple ()
@@ -88,8 +90,10 @@ bb0(%0 : $*P):
8890
%1 = alloc_stack $S
8991
checked_cast_addr_br take_on_success P in %0 : $*P to S in %1 : $*S, bb1, bb2
9092
bb1:
91-
br bb2
93+
br bb3
9294
bb2:
95+
br bb3
96+
bb3:
9397
destroy_addr %1 : $*S
9498
dealloc_stack %1 : $*S
9599
%2 = tuple ()
@@ -109,8 +113,10 @@ bb0(%0 : $*P):
109113
%1 = alloc_stack $S
110114
checked_cast_addr_br copy_on_success P in %0 : $*P to S in %1 : $*S, bb1, bb2
111115
bb1:
112-
br bb2
116+
br bb3
113117
bb2:
118+
br bb3
119+
bb3:
114120
destroy_addr %1 : $*S
115121
dealloc_stack %1 : $*S
116122
%2 = tuple ()

‎test/IRGen/enum.sil

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -568,12 +568,10 @@ entry(%u : $SinglePayloadNoXI2):
568568
// CHECK: ]
569569
// CHECK: ; <label>:[[DFLT]]
570570
// CHECK: unreachable
571-
switch_enum %u : $SinglePayloadNoXI2, case #SinglePayloadNoXI2.x!enumelt.1: x_dest, case #SinglePayloadNoXI2.y!enumelt: y_dest, case #SinglePayloadNoXI2.z!enumelt: z_dest, default end
571+
switch_enum %u : $SinglePayloadNoXI2, case #SinglePayloadNoXI2.x!enumelt.1: x_dest, case #SinglePayloadNoXI2.y!enumelt: y_dest, case #SinglePayloadNoXI2.z!enumelt: z_dest, default default_dest
572572

573573
// CHECK: ; <label>:[[X_PREDEST]]
574574
// CHECK: br label %[[X_DEST:[0-9]+]]
575-
// CHECK: ; <label>:[[SPLITEDGE]]
576-
// CHECK: br label %[[END:[0-9]+]]
577575
// CHECK: ; <label>:[[X_DEST]]
578576
// CHECK: {{%.*}} = phi [[WORD]] [ %0, %[[X_PREDEST]] ]
579577
x_dest(%u2 : $Builtin.Word):
@@ -599,6 +597,11 @@ z_dest:
599597
apply %c() : $@convention(thin) () -> ()
600598
br end
601599

600+
// CHECK: ; <label>:[[SPLITEDGE]]
601+
// CHECK: br label %[[END:[0-9]+]]
602+
default_dest:
603+
br end
604+
602605
// CHECK: ; <label>:[[END]]
603606
// CHECK: ret void
604607
end:
@@ -688,7 +691,7 @@ sil @int64_sink : $@convention(thin) (Builtin.Word) -> ()
688691
// CHECK: ([[WORD]], [[WORD]], i8) {{.*}} {
689692
sil @aggregate_single_payload_unpack : $@convention(thin) (AggregateSinglePayload) -> () {
690693
entry(%u : $AggregateSinglePayload):
691-
switch_enum %u : $AggregateSinglePayload, case #AggregateSinglePayload.x!enumelt.1: x_dest, default end
694+
switch_enum %u : $AggregateSinglePayload, case #AggregateSinglePayload.x!enumelt.1: x_dest, default default_dest
692695

693696
// CHECK: [[FIRST:%.*]] = phi [[WORD]] [ %0
694697
// CHECK: [[SECOND:%.*]] = phi [[WORD]] [ %1
@@ -702,6 +705,9 @@ x_dest(%v : $(Builtin.Word, Builtin.Word)):
702705
%d = apply %f(%b) : $@convention(thin) (Builtin.Word) -> ()
703706
br end
704707

708+
default_dest:
709+
br end
710+
705711
end:
706712
%x = tuple ()
707713
return %x : $()
@@ -745,7 +751,7 @@ enum AggregateSinglePayload2 {
745751
// CHECK-32: define{{( dllexport)?}}{{( protected)?}} swiftcc void @aggregate_single_payload_unpack_2(%T4enum23AggregateSinglePayload2O* noalias nocapture dereferenceable(16)) {{.*}} {
746752
sil @aggregate_single_payload_unpack_2 : $@convention(thin) (AggregateSinglePayload2) -> () {
747753
entry(%u : $AggregateSinglePayload2):
748-
switch_enum %u : $AggregateSinglePayload2, case #AggregateSinglePayload2.x!enumelt.1: x_dest, default end
754+
switch_enum %u : $AggregateSinglePayload2, case #AggregateSinglePayload2.x!enumelt.1: x_dest, default default_dest
749755

750756
// CHECK-64: [[TRUNC:%.*]] = trunc [[WORD]] %0 to i21
751757
// CHECK-64: phi i21 [ [[TRUNC]]
@@ -755,6 +761,9 @@ entry(%u : $AggregateSinglePayload2):
755761
x_dest(%v : $(CharLike, IntLike, RangeLike)):
756762
br end
757763

764+
default_dest:
765+
br end
766+
758767
end:
759768
%x = tuple ()
760769
return %x : $()
@@ -796,7 +805,7 @@ enum AggregateSinglePayload3 {
796805
// CHECK: define{{( dllexport)?}}{{( protected)?}} swiftcc void @aggregate_single_payload_unpack_3([[WORD]], [[WORD]]) {{.*}} {
797806
sil @aggregate_single_payload_unpack_3 : $@convention(thin) (AggregateSinglePayload3) -> () {
798807
entry(%u : $AggregateSinglePayload3):
799-
switch_enum %u : $AggregateSinglePayload3, case #AggregateSinglePayload3.x!enumelt.1: x_dest, default end
808+
switch_enum %u : $AggregateSinglePayload3, case #AggregateSinglePayload3.x!enumelt.1: x_dest, default default_dest
800809

801810
// CHECK: [[TRUNC:%.*]] = trunc [[WORD]] %0 to i21
802811
// CHECK: [[FIRST:%.*]] = phi i21 [ [[TRUNC]]
@@ -813,6 +822,9 @@ x_dest(%v : $(Builtin.Int21, Builtin.Word)):
813822
%d = apply %g(%b) : $@convention(thin) (Builtin.Word) -> ()
814823
br end
815824

825+
default_dest:
826+
br end
827+
816828
end:
817829
%x = tuple ()
818830
return %x : $()

‎test/IRGen/partial_apply.sil

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,8 +516,11 @@ bb(%0 : $Int):
516516

517517
bb1(%err: $Error):
518518
%t = tuple ()
519-
br bb2(%t: $())
519+
br bb3(%t: $())
520520

521-
bb2(%v : $()):
521+
bb2(%r : $()):
522+
br bb3(%r : $())
523+
524+
bb3(%v : $()):
522525
return %v : $()
523526
}

‎test/SIL/Parser/undef.sil

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -315,17 +315,25 @@ bb0:
315315

316316
sil @switch_enum_test : $() -> () {
317317
bb0:
318-
// CHECK: switch_enum undef : $E, case #E.Case!enumelt: bb1, default bb1
319-
switch_enum undef : $E, case #E.Case!enumelt: bb1, default bb1
318+
// CHECK: switch_enum undef : $E, case #E.Case!enumelt: bb1, default bb2
319+
switch_enum undef : $E, case #E.Case!enumelt: bb1, default bb2
320320
bb1:
321+
br bb3
322+
bb2:
323+
br bb3
324+
bb3:
321325
return undef : $()
322326
}
323327

324328
sil @switch_enum_addr_test : $() -> () {
325329
bb0:
326-
// CHECK: switch_enum_addr undef : $*E, case #E.Case!enumelt: bb1, default bb1
327-
switch_enum_addr undef : $*E, case #E.Case!enumelt: bb1, default bb1
330+
// CHECK: switch_enum_addr undef : $*E, case #E.Case!enumelt: bb1, default bb2
331+
switch_enum_addr undef : $*E, case #E.Case!enumelt: bb1, default bb2
328332
bb1:
333+
br bb3
334+
bb2:
335+
br bb3
336+
bb3:
329337
return undef : $()
330338
}
331339

@@ -334,8 +342,10 @@ bb0:
334342
// CHECK: dynamic_method_br undef : $P, #C.fn!1.foreign, bb1, bb2
335343
dynamic_method_br undef : $P, #C.fn!1.foreign, bb1, bb2
336344
bb1(%x : $@convention(objc_method) (P) -> ()):
337-
br bb2
345+
br bb3
338346
bb2:
347+
br bb3
348+
bb3:
339349
return undef: $()
340350
}
341351

@@ -344,8 +354,10 @@ bb0:
344354
// CHECK: checked_cast_br undef : $C to $C, bb1, bb2
345355
checked_cast_br undef : $C to $C, bb1, bb2
346356
bb1(%x : $C):
347-
br bb2
357+
br bb3
348358
bb2:
359+
br bb3
360+
bb3:
349361
return undef : $()
350362
}
351363

@@ -354,8 +366,10 @@ bb0:
354366
// CHECK: checked_cast_addr_br take_always C in undef : $*C to C in undef : $*C, bb1, bb2
355367
checked_cast_addr_br take_always C in undef : $*C to C in undef : $*C, bb1, bb2
356368
bb1:
357-
br bb2
369+
br bb3
358370
bb2:
371+
br bb3
372+
bb3:
359373
return undef : $()
360374
}
361375

‎test/SILGen/errors.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,7 @@ func supportFirstStructure<B: Buildable>(_ b: inout B) throws {
545545
// CHECK: [[BB_ERROR]]([[ERROR:%.*]] : @owned $Error):
546546
// CHECK: abort_apply [[TOKEN]]
547547
// CHECK: throw [[ERROR]]
548+
548549
// CHECK: } // end sil function '$S6errors21supportFirstStructure{{.*}}F'
549550

550551
func supportStructure<B: Buildable>(_ b: inout B, name: String) throws {
@@ -571,6 +572,7 @@ func supportStructure<B: Buildable>(_ b: inout B, name: String) throws {
571572
// CHECK: end_borrow [[BORROWED_INDEX_COPY]] from [[INDEX_COPY]]
572573
// CHECK: destroy_value [[INDEX_COPY]] : $String
573574
// CHECK: throw [[ERROR]]
575+
574576
// CHECK: } // end sil function '$S6errors16supportStructure{{.*}}F'
575577

576578
struct Pylon {

‎test/SILOptimizer/cse_objc.sil

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,28 +127,34 @@ bb0(%0 : $Walkable):
127127
// CHECK: open_existential_ref %0 : $AnyObject to $[[OPENED:@opened\(.*\) AnyObject]]
128128
// CHECK: bb1(%{{[0-9]*}} : $@convention(objc_method) ([[OPENED]]) -> @autoreleased Optional<NSString>):
129129
// CHECK-NOT: open_existential_ref
130-
// CHECK: bb2(%{{[0-9]*}} : $@convention(objc_method) ([[OPENED]]) -> @autoreleased Optional<NSString>):
130+
// CHECK: bb3(%{{[0-9]*}} : $@convention(objc_method) ([[OPENED]]) -> @autoreleased Optional<NSString>):
131131
// CHECK-NOT: open_existential_ref
132132
// CHECK: return
133133
sil @cse_open_existential_in_bbarg : $@convention(thin) (@owned AnyObject) -> () {
134134
bb0(%0 : $AnyObject):
135135
%1 = open_existential_ref %0 : $AnyObject to $@opened("CCEAC0E2-4BB2-11E6-86F8-B8E856428C60") AnyObject
136-
dynamic_method_br %1 : $@opened("CCEAC0E2-4BB2-11E6-86F8-B8E856428C60") AnyObject, #NSProxy.description!getter.1.foreign, bb1, bb3
136+
dynamic_method_br %1 : $@opened("CCEAC0E2-4BB2-11E6-86F8-B8E856428C60") AnyObject, #NSProxy.description!getter.1.foreign, bb1, bb2
137137

138138
bb1(%3 : $@convention(objc_method) (@opened("CCEAC0E2-4BB2-11E6-86F8-B8E856428C60") AnyObject) -> @autoreleased Optional<NSString>):
139139
strong_retain %1 : $@opened("CCEAC0E2-4BB2-11E6-86F8-B8E856428C60") AnyObject
140140
%5 = partial_apply %3(%1) : $@convention(objc_method) (@opened("CCEAC0E2-4BB2-11E6-86F8-B8E856428C60") AnyObject) -> @autoreleased Optional<NSString>
141141
%6 = apply %5() : $@callee_owned () -> @owned Optional<NSString>
142142
%7 = open_existential_ref %0 : $AnyObject to $@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject
143-
dynamic_method_br %7 : $@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject, #NSProxy.description!getter.1.foreign, bb2, bb3
143+
dynamic_method_br %7 : $@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject, #NSProxy.description!getter.1.foreign, bb3, bb4
144144

145-
bb2(%9 : $@convention(objc_method) (@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject) -> @autoreleased Optional<NSString>):
145+
bb2:
146+
br bb5
147+
148+
bb3(%9 : $@convention(objc_method) (@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject) -> @autoreleased Optional<NSString>):
146149
strong_retain %7 : $@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject
147150
%10 = partial_apply %9(%7) : $@convention(objc_method) (@opened("CCEDAF1E-4BB2-11E6-86F8-B8E856428C60") AnyObject) -> @autoreleased Optional<NSString>
148151
%11 = apply %10() : $@callee_owned () -> @owned Optional<NSString>
149-
br bb3
152+
br bb5
153+
154+
bb4:
155+
br bb5
150156

151-
bb3:
157+
bb5:
152158
strong_release %0 : $AnyObject
153159
%13 = tuple ()
154160
return %13 : $()

‎test/SILOptimizer/diagnose_unreachable.sil

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -395,39 +395,53 @@ sil_scope 1 { loc "f.swift":2:1 parent 0 }
395395
// CHECK: sil_scope [[S:[0-9]+]] { loc "f.swift":2:1 parent [[F]] }
396396
// CHECK-LABEL:sil @try_apply_2
397397
// CHECK: bb3({{.*}}$Never):
398+
// CHECK-NEXT: {{ unreachable }}, scope [[F]]
399+
// CHECK: bb5({{.*}}$Never):
398400
// CHECK-NEXT: {{ unreachable }}, scope [[S]]
399-
// CHECK: bb4(
401+
// CHECK: bb7({{.*}} : $Error):
400402
// CHECK-NEXT: throw
401-
403+
// CHECK-LABEL: } // end sil function 'try_apply_2'
402404
sil @try_apply_2 : $@convention(thin) (Builtin.Int1) -> @error Error {
403405
bb0(%0 : $Builtin.Int1):
404406
%1 = function_ref @throwing_noreturn : $@convention(thin) () -> (Never, @error Error)
405407
cond_br %0, bb1, bb2
406408
bb1:
407409
try_apply %1() : $@convention(thin) () -> (Never, @error Error), normal bb3, error bb4, scope 0
408410
bb2:
409-
try_apply %1() : $@convention(thin) () -> (Never, @error Error), normal bb3, error bb4, scope 1
411+
try_apply %1() : $@convention(thin) () -> (Never, @error Error), normal bb5, error bb6, scope 1
410412
bb3(%2 : $Never):
411-
%3 = tuple ()
412-
return %3 : $()
413+
br bb7
413414
bb4(%4 : $Error):
414-
throw %4 : $Error
415+
br bb8(%4 : $Error)
416+
bb5(%5 : $Never):
417+
br bb7
418+
bb6(%7 : $Error):
419+
br bb8(%7 : $Error)
420+
bb7:
421+
%8 = tuple ()
422+
return %8 : $()
423+
bb8(%9 : $Error):
424+
throw %9 : $Error
415425
}
416426

417-
// This should arguably be ill-formed.
418427
// CHECK-LABEL:sil @try_apply_3
419428
// CHECK: br bb1
420-
// CHECK: bb1(
429+
// CHECK: bb1:
421430
// CHECK-NOT: {{ unreachable}}
422431
// CHECK: try_apply
423-
// CHECK: bb2(
432+
// CHECK: bb3({{.*}} : $Error):
424433
// CHECK-NEXT: throw
425434
sil @try_apply_3 : $@convention(thin) (Never) -> @error Error {
426435
bb0(%0 : $Never):
427436
br bb1(%0 : $Never)
437+
428438
bb1(%1 : $Never):
429439
%2 = function_ref @throwing_noreturn : $@convention(thin) () -> (Never, @error Error)
430-
try_apply %2() : $@convention(thin) () -> (Never, @error Error), normal bb1, error bb2
431-
bb2(%3 : $Error):
432-
throw %3 : $Error
440+
try_apply %2() : $@convention(thin) () -> (Never, @error Error), normal bb2, error bb3
441+
442+
bb2(%3 : $Never):
443+
br bb1(%3 : $Never)
444+
445+
bb3(%4 : $Error):
446+
throw %4 : $Error
433447
}

0 commit comments

Comments
 (0)