Skip to content

Commit c8284b0

Browse files
swiftixswiftix
committed
Revert "[sil-serializer] Do not use RPOT order for serializing SIL basic blocks"
This reverts commit 2f9ee20. It broke some of the stdlib serialization tests. rdar://problem/27404044
1 parent 425138c commit c8284b0

File tree

8 files changed

+202
-247
lines changed

8 files changed

+202
-247
lines changed

‎include/swift/SIL/SILOpenedArchetypesTracker.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,11 @@ class SILOpenedArchetypesTracker : public DeleteNotificationHandler {
4949

5050
const SILFunction &getFunction() const { return F; }
5151

52-
// Register a definiton of a given opened archetype.
53-
void addOpenedArchetypeDef(Type archetype, SILValue Def);
52+
void addOpenedArchetypeDef(Type archetype, SILValue Def) {
53+
assert(!getOpenedArchetypeDef(archetype) &&
54+
"There can be only one definition of an opened archetype");
55+
OpenedArchetypeDefs[archetype] = Def;
56+
}
5457

5558
void removeOpenedArchetypeDef(Type archetype, SILValue Def) {
5659
auto FoundDef = getOpenedArchetypeDef(archetype);

‎lib/SIL/SILOpenedArchetypesTracker.cpp

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,6 @@
1414

1515
using namespace swift;
1616

17-
void SILOpenedArchetypesTracker::addOpenedArchetypeDef(Type archetype,
18-
SILValue Def) {
19-
auto OldDef = getOpenedArchetypeDef(archetype);
20-
if (OldDef && isa<GlobalAddrInst>(OldDef)) {
21-
// It is a forward definition created during deserialization.
22-
// Replace it with the real definition now.
23-
OldDef->replaceAllUsesWith(Def);
24-
OldDef = SILValue();
25-
}
26-
assert(!OldDef &&
27-
"There can be only one definition of an opened archetype");
28-
OpenedArchetypeDefs[archetype] = Def;
29-
}
30-
3117
// Register archetypes opened by a given instruction.
3218
// Can be used to incrementally populate the mapping, e.g.
3319
// if it is done when performing a scan of all instructions

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 167 additions & 205 deletions
Large diffs are not rendered by default.

‎lib/Serialization/DeserializeSIL.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,6 @@ namespace swift {
108108
readDefaultWitnessTable(serialization::DeclID,
109109
SILDefaultWitnessTable *existingWt);
110110

111-
/// A helper method to get a type based on a TypeID.
112-
/// If this type is an opened archetype and its
113-
/// definition is not seen yet, create a placeholder
114-
/// SILValue representing a forward definition and
115-
/// register it as a definition of this opened archetype.
116-
Type getType(SILBuilder &Builder, ModuleFile *MF,
117-
serialization::TypeID TID);
118-
119111
public:
120112
Identifier getModuleIdentifier() const {
121113
return MF->getAssociatedModule()->getName();

‎lib/Serialization/SerializeSIL.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,20 +392,31 @@ void SILSerializer::writeSILFunction(const SILFunction &F, bool DeclOnly) {
392392
// Assign a value ID to each SILInstruction that has value and to each basic
393393
// block argument.
394394
unsigned ValueID = 0;
395-
for (const auto &BB : F) {
395+
llvm::ReversePostOrderTraversal<SILFunction *> RPOT(
396+
const_cast<SILFunction *>(&F));
397+
for (auto Iter = RPOT.begin(), E = RPOT.end(); Iter != E; ++Iter) {
398+
auto &BB = **Iter;
396399
BasicBlockMap.insert(std::make_pair(&BB, BasicID++));
397400

398401
for (auto I = BB.bbarg_begin(), E = BB.bbarg_end(); I != E; ++I)
399402
ValueIDs[static_cast<const ValueBase*>(*I)] = ++ValueID;
400403

401-
for (const auto &SI : BB)
404+
for (const SILInstruction &SI : BB)
402405
if (SI.hasValue())
403406
ValueIDs[&SI] = ++ValueID;
404407
}
405408

406-
for (const auto &BB : F) {
407-
writeSILBasicBlock(BB);
409+
// Write SIL basic blocks in the RPOT order
410+
// to make sure that instructions defining open archetypes
411+
// are serialized before instructions using those opened
412+
// archetypes.
413+
unsigned SerializedBBNum = 0;
414+
for (auto Iter = RPOT.begin(), E = RPOT.end(); Iter != E; ++Iter) {
415+
auto *BB = *Iter;
416+
writeSILBasicBlock(*BB);
417+
SerializedBBNum++;
408418
}
419+
assert(BasicID == SerializedBBNum && "Wrong number of BBs was serialized");
409420
}
410421

411422
void SILSerializer::writeSILBasicBlock(const SILBasicBlock &BB) {

‎test/ClangModules/serialization-sil.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,36 @@ public func testPartialApply(_ obj: Test) {
1010
// CHECK-LABEL: @_TF4Test16testPartialApplyFPSo4Test_T_ : $@convention(thin) (@owned Test) -> () {
1111
if let curried1 = obj.normalObject {
1212
// CHECK: dynamic_method_br [[CURRIED1_OBJ:%.+]] : $@opened([[CURRIED1_EXISTENTIAL:.+]]) Test, #Test.normalObject!1.foreign, [[CURRIED1_TRUE:[^,]+]], [[CURRIED1_FALSE:[^,]+]]
13+
// CHECK: [[CURRIED1_FALSE]]:
1314
// CHECK: [[CURRIED1_TRUE]]([[CURRIED1_METHOD:%.+]] : $@convention(objc_method) (@opened([[CURRIED1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject):
1415
// CHECK: [[CURRIED1_PARTIAL:%.+]] = partial_apply [[CURRIED1_METHOD]]([[CURRIED1_OBJ]]) : $@convention(objc_method) (@opened([[CURRIED1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject
1516
// CHECK: [[CURRIED1_THUNK:%.+]] = function_ref @_TTRXFo__oPs9AnyObject__XFo_iT__iPS___ : $@convention(thin) (@in (), @owned @callee_owned () -> @owned AnyObject) -> @out AnyObject
1617
// CHECK: = partial_apply [[CURRIED1_THUNK]]([[CURRIED1_PARTIAL]]) : $@convention(thin) (@in (), @owned @callee_owned () -> @owned AnyObject) -> @out AnyObject
17-
// CHECK: [[CURRIED1_FALSE]]:
1818
curried1()
1919
}
2020
if let curried2 = obj.innerPointer {
2121
// CHECK: dynamic_method_br [[CURRIED2_OBJ:%.+]] : $@opened([[CURRIED2_EXISTENTIAL:.+]]) Test, #Test.innerPointer!1.foreign, [[CURRIED2_TRUE:[^,]+]], [[CURRIED2_FALSE:[^,]+]]
22+
// CHECK: [[CURRIED2_FALSE]]:
2223
// CHECK: [[CURRIED2_TRUE]]([[CURRIED2_METHOD:%.+]] : $@convention(objc_method) (@opened([[CURRIED2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>):
2324
// CHECK: [[CURRIED2_PARTIAL:%.+]] = partial_apply [[CURRIED2_METHOD]]([[CURRIED2_OBJ]]) : $@convention(objc_method) (@opened([[CURRIED2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>
2425
// CHECK: [[CURRIED2_THUNK:%.+]] = function_ref @_TTRXFo__dGSpT___XFo_iT__iGSpT___ : $@convention(thin) (@in (), @owned @callee_owned () -> UnsafeMutablePointer<()>) -> @out UnsafeMutablePointer<()>
2526
// CHECK: = partial_apply [[CURRIED2_THUNK]]([[CURRIED2_PARTIAL]]) : $@convention(thin) (@in (), @owned @callee_owned () -> UnsafeMutablePointer<()>) -> @out UnsafeMutablePointer<()>
26-
// CHECK: [[CURRIED2_FALSE]]:
2727
curried2()
2828
}
2929
if let prop1 = obj.normalObjectProp {
3030
// CHECK: dynamic_method_br [[PROP1_OBJ:%.+]] : $@opened([[PROP1_EXISTENTIAL:.+]]) Test, #Test.normalObjectProp!getter.1.foreign, [[PROP1_TRUE:[^,]+]], [[PROP1_FALSE:[^,]+]]
31+
// CHECK: [[PROP1_FALSE]]:
3132
// CHECK: [[PROP1_TRUE]]([[PROP1_METHOD:%.+]] : $@convention(objc_method) (@opened([[PROP1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject):
3233
// CHECK: [[PROP1_PARTIAL:%.+]] = partial_apply [[PROP1_METHOD]]([[PROP1_OBJ]]) : $@convention(objc_method) (@opened([[PROP1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject
3334
// CHECK: = apply [[PROP1_PARTIAL]]() : $@callee_owned () -> @owned AnyObject
34-
// CHECK: [[PROP1_FALSE]]:
3535
_ = prop1
3636
}
3737
if let prop2 = obj.innerPointerProp {
3838
// CHECK: dynamic_method_br [[PROP2_OBJ:%.+]] : $@opened([[PROP2_EXISTENTIAL:.+]]) Test, #Test.innerPointerProp!getter.1.foreign, [[PROP2_TRUE:[^,]+]], [[PROP2_FALSE:[^,]+]]
39+
// CHECK: [[PROP2_FALSE]]:
3940
// CHECK: [[PROP2_TRUE]]([[PROP2_METHOD:%.+]] : $@convention(objc_method) (@opened([[PROP2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>):
4041
// CHECK: [[PROP2_PARTIAL:%.+]] = partial_apply [[PROP2_METHOD]]([[PROP2_OBJ]]) : $@convention(objc_method) (@opened([[PROP2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>
4142
// CHECK: = apply [[PROP2_PARTIAL]]() : $@callee_owned () -> UnsafeMutablePointer<()>
42-
// CHECK: [[PROP2_FALSE]]:
4343
_ = prop2
4444
}
4545
} // CHECK: {{^}$}}

‎test/Serialization/Inputs/def_basic.sil

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ bb1:
7777
// CHECK-LABEL: sil public_external [fragile] @test2 : $@convention(thin) (Int) -> ()
7878
sil [fragile] @test2 : $@convention(thin) (Int) -> () {
7979
// CHECK: bb1:
80-
// CHECK: return %5 : $()
80+
// CHECK: %[[VAL:[0-9]+]] = tuple ()
81+
// CHECK: br bb2
8182
// CHECK: bb2:
82-
// CHECK: %5 = tuple ()
83-
// CHECK: br bb1
83+
// CHECK: return %[[VAL]] : $()
8484
bb0(%0 : $Int):
8585
br bb2
8686
bb1:
@@ -602,7 +602,7 @@ bb3(%6 : $Builtin.Word):
602602
sil [fragile] @test_cond_branch_basic_block_args : $@convention(thin) (Int, Builtin.Int1) -> Int {
603603
bb0(%0 : $Int, %1 : $Builtin.Int1):
604604
cond_br %1, bb1(%0 : $Int), bb2(%0 : $Int)
605-
// CHECK: cond_br %1, bb1(%0 : $Int), bb2(%0 : $Int)
605+
// CHECK: cond_br %1, bb2(%0 : $Int), bb1(%0 : $Int)
606606
bb1(%3 : $Int):
607607
br bb3 (%3 : $Int)
608608
bb2(%2 : $Int):
@@ -716,12 +716,13 @@ bb1:
716716
bb2:
717717
%7 = function_ref @_TF6switch1aFT_T_ : $@convention(thin) () -> ()
718718
%8 = apply %7() : $@convention(thin) () -> ()
719-
br bb5 // CHECK: br
719+
br bb5
720720

721721
bb3(%10 : $Int):
722722
// CHECK: unchecked_enum_data {{%.*}} : $MaybePair, #MaybePair.Left!enumelt.1
723723
%x = unchecked_enum_data %3 : $MaybePair, #MaybePair.Left!enumelt.1
724724
br bb4(%x : $Int)
725+
// CHECK: br
725726

726727
bb4(%y : $Int):
727728
%12 = function_ref @_TF6switch1bFT_T_ : $@convention(thin) () -> ()
@@ -763,7 +764,7 @@ bb3:
763764
// CHECK-LABEL: sil public_external [fragile] @test_switch_value : $@convention(thin) (Builtin.Word) -> ()
764765
sil [fragile] @test_switch_value : $@convention(thin) (Builtin.Word) -> () {
765766
bb0(%0 : $Builtin.Word):
766-
// CHECK: switch_value %{{.*}} : $Builtin.Word, case %1: bb1, case %2: bb2
767+
// CHECK: switch_value %{{.*}} : $Builtin.Word, case %1: bb2, case %2: bb1
767768
%1 = integer_literal $Builtin.Word, 1
768769
%2 = integer_literal $Builtin.Word, 2
769770
switch_value %0 : $Builtin.Word, case %1: bb1, case %2: bb2
@@ -1016,7 +1017,7 @@ sil [fragile] @block_invoke : $@convention(c) (@inout_aliasable @block_storage I
10161017
// CHECK-LABEL: sil public_external [fragile] @test_try_apply : $@convention(thin) (@convention(thin) () -> @error Error) -> @error Error {
10171018
sil [fragile] @test_try_apply : $@convention(thin) (@convention(thin) () -> @error Error) -> @error Error {
10181019
bb0(%0 : $@convention(thin) () -> @error Error):
1019-
// CHECK: try_apply %0() : $@convention(thin) () -> @error Error, normal bb1, error bb2
1020+
// CHECK: try_apply %0() : $@convention(thin) () -> @error Error, normal bb2, error bb1
10201021
try_apply %0() : $@convention(thin) () -> @error Error, normal bb1, error bb2
10211022

10221023
bb1(%1 : $()):

‎test/Serialization/transparent.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ func wrap_br() {
4444
// SIL: bb0(%0 : $MaybePair):
4545
// SIL: retain_value %0 : $MaybePair
4646
// SIL: switch_enum %0 : $MaybePair, case #MaybePair.Neither!enumelt: bb[[CASE1:[0-9]+]], case #MaybePair.Left!enumelt.1: bb[[CASE2:[0-9]+]], case #MaybePair.Right!enumelt.1: bb[[CASE3:[0-9]+]], case #MaybePair.Both!enumelt.1: bb[[CASE4:[0-9]+]]
47-
// SIL: bb[[CASE1]]:
48-
// SIL: bb[[CASE2]](%{{.*}} : $Int32):
49-
// SIL: bb[[CASE3]](%{{.*}} : $String):
5047
// SIL: bb[[CASE4]](%{{.*}} : $(Int32, String)):
48+
// SIL: bb[[CASE3]](%{{.*}} : $String):
49+
// SIL: bb[[CASE2]](%{{.*}} : $Int32):
50+
// SIL: bb[[CASE1]]:
5151
func test_switch(u: MaybePair) {
5252
do_switch(u: u)
5353
}

0 commit comments

Comments
 (0)