[sil-serializer] Do not use RPOT order for serializing SIL basic blocks

swiftix · swiftix · commit 2f9ee20df2f7 · 2016-07-16T21:53:17.000-07:00
My earlier patch started serializing SIL basic blocks using the RPOT order. While it works, changing the existing order of BBs during the serialization may be very surprising for users. After all, serialization is not supposed to transform the code.

Therefore, this patch follows a different approach. It uses the existing order of BBs during the serialization. When it deserializes SIL and detects a use of an opened archetype before its definition, it basically introduced a forward definition of this opened archetype. Later on, when the actual definition of the opened archetype is found, it replaces the forward definition. There is a correctness check at the end of a SIL function deserialization, which verifies that there are no forward definitions of opened archetypes left unresoved.
diff --git a/include/swift/SIL/SILOpenedArchetypesTracker.h b/include/swift/SIL/SILOpenedArchetypesTracker.h
@@ -49,11 +49,8 @@ class SILOpenedArchetypesTracker : public DeleteNotificationHandler {
 
   const SILFunction &getFunction() const { return F; }
 
-  void addOpenedArchetypeDef(Type archetype, SILValue Def) {
-    assert(!getOpenedArchetypeDef(archetype) &&
-           "There can be only one definition of an opened archetype");
-    OpenedArchetypeDefs[archetype] = Def;
-  }
+  // Register a definiton of a given opened archetype.
+  void addOpenedArchetypeDef(Type archetype, SILValue Def);
 
   void removeOpenedArchetypeDef(Type archetype, SILValue Def) {
     auto FoundDef = getOpenedArchetypeDef(archetype);
diff --git a/lib/SIL/SILOpenedArchetypesTracker.cpp b/lib/SIL/SILOpenedArchetypesTracker.cpp
@@ -14,6 +14,20 @@
 
 using namespace swift;
 
+void SILOpenedArchetypesTracker::addOpenedArchetypeDef(Type archetype,
+                                                       SILValue Def) {
+  auto OldDef = getOpenedArchetypeDef(archetype);
+  if (OldDef && isa<GlobalAddrInst>(OldDef)) {
+    // It is a forward definition created during deserialization.
+    // Replace it with the real definition now.
+    OldDef->replaceAllUsesWith(Def);
+    OldDef = SILValue();
+  }
+  assert(!OldDef &&
+         "There can be only one definition of an opened archetype");
+  OpenedArchetypeDefs[archetype] = Def;
+}
+
 // Register archetypes opened by a given instruction.
 // Can be used to incrementally populate the mapping, e.g.
 // if it is done when performing a scan of all instructions
diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp
diff --git a/lib/Serialization/DeserializeSIL.h b/lib/Serialization/DeserializeSIL.h
@@ -108,6 +108,14 @@ namespace swift {
     readDefaultWitnessTable(serialization::DeclID,
                             SILDefaultWitnessTable *existingWt);
 
+    /// A helper method to get a type based on a TypeID.
+    /// If this type is an opened archetype and its
+    /// definition is not seen yet, create a placeholder
+    /// SILValue representing a forward definition and
+    /// register it as a definition of this opened archetype.
+    Type getType(SILBuilder &Builder, ModuleFile *MF,
+                 serialization::TypeID TID);
+
 public:
     Identifier getModuleIdentifier() const {
       return MF->getAssociatedModule()->getName();
diff --git a/lib/Serialization/SerializeSIL.cpp b/lib/Serialization/SerializeSIL.cpp
@@ -392,31 +392,20 @@ void SILSerializer::writeSILFunction(const SILFunction &F, bool DeclOnly) {
   // Assign a value ID to each SILInstruction that has value and to each basic
   // block argument.
   unsigned ValueID = 0;
-  llvm::ReversePostOrderTraversal<SILFunction *> RPOT(
-      const_cast<SILFunction *>(&F));
-  for (auto Iter = RPOT.begin(), E = RPOT.end(); Iter != E; ++Iter) {
-    auto &BB = **Iter;
+  for (const auto &BB : F) {
     BasicBlockMap.insert(std::make_pair(&BB, BasicID++));
 
     for (auto I = BB.bbarg_begin(), E = BB.bbarg_end(); I != E; ++I)
       ValueIDs[static_cast<const ValueBase*>(*I)] = ++ValueID;
 
-    for (const SILInstruction &SI : BB)
+    for (const auto &SI : BB)
       if (SI.hasValue())
         ValueIDs[&SI] = ++ValueID;
   }
 
-  // Write SIL basic blocks in the RPOT order
-  // to make sure that instructions defining open archetypes
-  // are serialized before instructions using those opened
-  // archetypes.
-  unsigned SerializedBBNum = 0;
-  for (auto Iter = RPOT.begin(), E = RPOT.end(); Iter != E; ++Iter) {
-    auto *BB = *Iter;
-    writeSILBasicBlock(*BB);
-    SerializedBBNum++;
+  for (const auto &BB : F) {
+    writeSILBasicBlock(BB);
   }
-  assert(BasicID == SerializedBBNum && "Wrong number of BBs was serialized");
 }
 
 void SILSerializer::writeSILBasicBlock(const SILBasicBlock &BB) {
diff --git a/test/ClangModules/serialization-sil.swift b/test/ClangModules/serialization-sil.swift
@@ -10,36 +10,36 @@ public func testPartialApply(_ obj: Test) {
   // CHECK-LABEL: @_TF4Test16testPartialApplyFPSo4Test_T_ : $@convention(thin) (@owned Test) -> () {
   if let curried1 = obj.normalObject {
     // CHECK: dynamic_method_br [[CURRIED1_OBJ:%.+]] : $@opened([[CURRIED1_EXISTENTIAL:.+]]) Test, #Test.normalObject!1.foreign, [[CURRIED1_TRUE:[^,]+]], [[CURRIED1_FALSE:[^,]+]]
-    // CHECK: [[CURRIED1_FALSE]]:
     // CHECK: [[CURRIED1_TRUE]]([[CURRIED1_METHOD:%.+]] : $@convention(objc_method) (@opened([[CURRIED1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject):
     // CHECK: [[CURRIED1_PARTIAL:%.+]] = partial_apply [[CURRIED1_METHOD]]([[CURRIED1_OBJ]]) : $@convention(objc_method) (@opened([[CURRIED1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject
     // CHECK: [[CURRIED1_THUNK:%.+]] = function_ref @_TTRXFo__oPs9AnyObject__XFo_iT__iPS___ : $@convention(thin) (@in (), @owned @callee_owned () -> @owned AnyObject) -> @out AnyObject
     // CHECK: = partial_apply [[CURRIED1_THUNK]]([[CURRIED1_PARTIAL]]) : $@convention(thin) (@in (), @owned @callee_owned () -> @owned AnyObject) -> @out AnyObject
+    // CHECK: [[CURRIED1_FALSE]]:
     curried1()
   }
   if let curried2 = obj.innerPointer {
     // CHECK: dynamic_method_br [[CURRIED2_OBJ:%.+]] : $@opened([[CURRIED2_EXISTENTIAL:.+]]) Test, #Test.innerPointer!1.foreign, [[CURRIED2_TRUE:[^,]+]], [[CURRIED2_FALSE:[^,]+]]
-    // CHECK: [[CURRIED2_FALSE]]:
     // CHECK: [[CURRIED2_TRUE]]([[CURRIED2_METHOD:%.+]] : $@convention(objc_method) (@opened([[CURRIED2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>):
     // CHECK: [[CURRIED2_PARTIAL:%.+]] = partial_apply [[CURRIED2_METHOD]]([[CURRIED2_OBJ]]) : $@convention(objc_method) (@opened([[CURRIED2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>
     // CHECK: [[CURRIED2_THUNK:%.+]] = function_ref @_TTRXFo__dGSpT___XFo_iT__iGSpT___ : $@convention(thin) (@in (), @owned @callee_owned () -> UnsafeMutablePointer<()>) -> @out UnsafeMutablePointer<()>
     // CHECK: = partial_apply [[CURRIED2_THUNK]]([[CURRIED2_PARTIAL]]) : $@convention(thin) (@in (), @owned @callee_owned () -> UnsafeMutablePointer<()>) -> @out UnsafeMutablePointer<()>
+    // CHECK: [[CURRIED2_FALSE]]:
     curried2()
   }
   if let prop1 = obj.normalObjectProp {
     // CHECK: dynamic_method_br [[PROP1_OBJ:%.+]] : $@opened([[PROP1_EXISTENTIAL:.+]]) Test, #Test.normalObjectProp!getter.1.foreign, [[PROP1_TRUE:[^,]+]], [[PROP1_FALSE:[^,]+]]
-    // CHECK: [[PROP1_FALSE]]:
     // CHECK: [[PROP1_TRUE]]([[PROP1_METHOD:%.+]] : $@convention(objc_method) (@opened([[PROP1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject):
     // CHECK: [[PROP1_PARTIAL:%.+]] = partial_apply [[PROP1_METHOD]]([[PROP1_OBJ]]) : $@convention(objc_method) (@opened([[PROP1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject
     // CHECK: = apply [[PROP1_PARTIAL]]() : $@callee_owned () -> @owned AnyObject
+    // CHECK: [[PROP1_FALSE]]:
     _ = prop1
   }
   if let prop2 = obj.innerPointerProp {
     // CHECK: dynamic_method_br [[PROP2_OBJ:%.+]] : $@opened([[PROP2_EXISTENTIAL:.+]]) Test, #Test.innerPointerProp!getter.1.foreign, [[PROP2_TRUE:[^,]+]], [[PROP2_FALSE:[^,]+]]
-    // CHECK: [[PROP2_FALSE]]:
     // CHECK: [[PROP2_TRUE]]([[PROP2_METHOD:%.+]] : $@convention(objc_method) (@opened([[PROP2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>):
     // CHECK: [[PROP2_PARTIAL:%.+]] = partial_apply [[PROP2_METHOD]]([[PROP2_OBJ]]) : $@convention(objc_method) (@opened([[PROP2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>
     // CHECK: = apply [[PROP2_PARTIAL]]() : $@callee_owned () -> UnsafeMutablePointer<()>
+    // CHECK: [[PROP2_FALSE]]:
     _ = prop2
   }
 } // CHECK: {{^}$}}
diff --git a/test/Serialization/Inputs/def_basic.sil b/test/Serialization/Inputs/def_basic.sil
@@ -77,10 +77,10 @@ bb1:
 // CHECK-LABEL: sil public_external [fragile] @test2 : $@convention(thin) (Int) -> ()
 sil [fragile] @test2 : $@convention(thin) (Int) -> () {
   // CHECK: bb1:
-  // CHECK: %[[VAL:[0-9]+]] = tuple ()
-  // CHECK: br bb2
+  // CHECK: return %5 : $()
   // CHECK: bb2:
-  // CHECK: return %[[VAL]] : $()
+  // CHECK: %5 = tuple ()
+  // CHECK: br bb1
 bb0(%0 : $Int):
   br bb2
 bb1:
@@ -602,7 +602,7 @@ bb3(%6 : $Builtin.Word):
 sil [fragile] @test_cond_branch_basic_block_args : $@convention(thin) (Int, Builtin.Int1) -> Int {
 bb0(%0 : $Int, %1 : $Builtin.Int1):
   cond_br %1, bb1(%0 : $Int), bb2(%0 : $Int)
-  // CHECK: cond_br %1, bb2(%0 : $Int), bb1(%0 : $Int)
+  // CHECK: cond_br %1, bb1(%0 : $Int), bb2(%0 : $Int)
 bb1(%3 : $Int):
   br bb3 (%3 : $Int)
 bb2(%2 : $Int):
@@ -716,13 +716,12 @@ bb1:
 bb2:
   %7 = function_ref @_TF6switch1aFT_T_ : $@convention(thin) () -> ()
   %8 = apply %7() : $@convention(thin) () -> ()
-  br bb5
+  br bb5                                                 // CHECK: br
 
 bb3(%10 : $Int):
   // CHECK: unchecked_enum_data {{%.*}} : $MaybePair, #MaybePair.Left!enumelt.1
   %x = unchecked_enum_data %3 : $MaybePair, #MaybePair.Left!enumelt.1
   br bb4(%x : $Int)
-  // CHECK: br
 
 bb4(%y : $Int):
   %12 = function_ref @_TF6switch1bFT_T_ : $@convention(thin) () -> ()
@@ -764,7 +763,7 @@ bb3:
 // CHECK-LABEL: sil public_external [fragile] @test_switch_value : $@convention(thin) (Builtin.Word) -> ()
 sil [fragile] @test_switch_value : $@convention(thin) (Builtin.Word) -> () {
 bb0(%0 : $Builtin.Word):
-  // CHECK: switch_value %{{.*}} : $Builtin.Word, case %1: bb2, case %2: bb1
+  // CHECK: switch_value %{{.*}} : $Builtin.Word, case %1: bb1, case %2: bb2
   %1 = integer_literal $Builtin.Word, 1
   %2 = integer_literal $Builtin.Word, 2
   switch_value %0 : $Builtin.Word, case %1: bb1, case %2: bb2
@@ -1017,7 +1016,7 @@ sil [fragile] @block_invoke : $@convention(c) (@inout_aliasable @block_storage I
 // CHECK-LABEL: sil public_external [fragile] @test_try_apply : $@convention(thin) (@convention(thin) () -> @error Error) -> @error Error {
 sil [fragile]  @test_try_apply : $@convention(thin) (@convention(thin) () -> @error Error) -> @error Error {
 bb0(%0 : $@convention(thin) () -> @error Error):
-  // CHECK: try_apply %0() : $@convention(thin) () -> @error Error, normal bb2, error bb1
+  // CHECK: try_apply %0() : $@convention(thin) () -> @error Error, normal bb1, error bb2
   try_apply %0() : $@convention(thin) () -> @error Error, normal bb1, error bb2
 
 bb1(%1 : $()):
diff --git a/test/Serialization/transparent.swift b/test/Serialization/transparent.swift
@@ -44,10 +44,10 @@ func wrap_br() {
 // SIL: bb0(%0 : $MaybePair):
 // SIL: retain_value %0 : $MaybePair
 // 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]+]]
-// SIL: bb[[CASE4]](%{{.*}} : $(Int32, String)):
-// SIL: bb[[CASE3]](%{{.*}} : $String):
-// SIL: bb[[CASE2]](%{{.*}} : $Int32):
 // SIL: bb[[CASE1]]:
+// SIL: bb[[CASE2]](%{{.*}} : $Int32):
+// SIL: bb[[CASE3]](%{{.*}} : $String):
+// SIL: bb[[CASE4]](%{{.*}} : $(Int32, String)):
 func test_switch(u: MaybePair) {
   do_switch(u: u)
 }
