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

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/parses 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 unresolved.

Author: swiftix

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);
@@ -83,10 +80,19 @@ class SILOpenedArchetypesTracker : public DeleteNotificationHandler {
   // the typedef operands of this instruction.
   void registerUsedOpenedArchetypes(const SILInstruction *I);
 
+  // Register opened archetypes referenced by this type, if they
+  // are not registered yet. Create placeholders representing forward
+  // definitions of these opened archetypes.
+  void registerUsedOpenedArchetypes(Type Ty);
+
   // Unregister archetypes opened by a given instruction.
   // Should be only called when this instruction is to be removed.
   void unregisterOpenedArchetypes(const SILInstruction *I);
 
+  // Returns true of some of the forward opened archetype definitions
+  // are unresolved.
+  bool hasUnresolvedOpenedArchetypeDefinitions();
+
   // Handling of instruction removal notifications.
   bool needsNotifications() { return true; }
 

include/swift/Serialization/ModuleFile.h

@@ -43,6 +43,7 @@ class ProtocolConformance;
 /// A serialized module, along with the tools to access it.
 class ModuleFile : public LazyMemberLoader {
   friend class SerializedASTFile;
+  friend class SILDeserializer;
   using Status = serialization::Status;
 
   /// A reference back to the AST representation of the file.
@@ -76,6 +77,9 @@ class ModuleFile : public LazyMemberLoader {
   /// The data blob containing all of the module's identifiers.
   StringRef IdentifierData;
 
+  /// A callback to be invoked every time a type was deserialized.
+  llvm::function_ref<void(Type)> DeserializedTypeCallback;
+
 public:
   /// Represents another module that has been imported as a dependency.
   class Dependency {

lib/Parse/ParseSIL.cpp

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "swift/Basic/Defer.h"
 #include "swift/Basic/Fallthrough.h"
 #include "swift/AST/ArchetypeBuilder.h"
 #include "swift/AST/NameLookup.h"
@@ -94,6 +95,7 @@ namespace {
   };
 
   class SILParser {
+    friend Parser;
   public:
     Parser &P;
     SILModule &SILMod;
@@ -112,14 +114,19 @@ namespace {
     llvm::StringMap<ValueBase*> LocalValues;
     llvm::StringMap<SourceLoc> ForwardRefLocalValues;
 
+    /// A callback to be invoked every time a type was deserialized.
+    llvm::function_ref<void(Type)> ParsedTypeCallback;
+
+
     bool performTypeLocChecking(TypeLoc &T, bool IsSIL = true);
     bool parseSpecConformanceSubstitutions(
                    SmallVectorImpl<ParsedSubstitution> &parsed);
     ProtocolConformance *parseProtocolConformanceHelper(ProtocolDecl *&proto,
                                                         bool localScope);
   public:
-
-    SILParser(Parser &P) : P(P), SILMod(*P.SIL->M), TUState(*P.SIL->S) {}
+    SILParser(Parser &P)
+        : P(P), SILMod(*P.SIL->M), TUState(*P.SIL->S),
+          ParsedTypeCallback([](Type ty) {}) {}
 
     /// diagnoseProblems - After a function is fully parse, emit any diagnostics
     /// for errors and return true if there were any.
@@ -796,6 +803,8 @@ bool SILParser::parseASTType(CanType &result) {
   if (performTypeLocChecking(loc, false))
     return true;
   result = loc.getType()->getCanonicalType();
+  // Invoke the callback on the parsed type.
+  ParsedTypeCallback(loc.getType());
   return false;
 }
 
@@ -838,6 +847,10 @@ bool SILParser::parseSILTypeWithoutQualifiers(SILType &Result,
 
   Result = SILType::getPrimitiveType(Ty.getType()->getCanonicalType(),
                                      category);
+
+  // Invoke the callback on the parsed type.
+  ParsedTypeCallback(Ty.getType());
+
   return false;
 
 }
@@ -3922,6 +3935,17 @@ bool Parser::parseDeclSIL() {
       // is required for adding typedef operands to instructions.
       B.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
 
+      // Define a callback to be invoked on the deserialized types.
+      auto OldParsedTypeCallback = FunctionState.ParsedTypeCallback;
+      SWIFT_DEFER {
+        FunctionState.ParsedTypeCallback = OldParsedTypeCallback;
+      };
+
+      FunctionState.ParsedTypeCallback = [&OpenedArchetypesTracker,
+                                          &FunctionState](Type ty) {
+        OpenedArchetypesTracker.registerUsedOpenedArchetypes(ty);
+      };
+
       do {
         if (FunctionState.parseSILBasicBlock(B))
           return true;
@@ -3930,6 +3954,12 @@ bool Parser::parseDeclSIL() {
       SourceLoc RBraceLoc;
       parseMatchingToken(tok::r_brace, RBraceLoc, diag::expected_sil_rbrace,
                          LBraceLoc);
+
+      // Check that there are no unresolved forward definitions of opened
+      // archetypes.
+      if (OpenedArchetypesTracker.hasUnresolvedOpenedArchetypeDefinitions())
+        llvm_unreachable(
+            "All forward definitions of opened archetypes should be resolved");
     }
 
     FunctionState.F->setLinkage(resolveSILLinkage(FnLinkage, isDefinition));

lib/SIL/SILOpenedArchetypesTracker.cpp

@@ -14,6 +14,60 @@
 
 using namespace swift;
 
+void SILOpenedArchetypesTracker::addOpenedArchetypeDef(Type archetype,
+                                                       SILValue Def) {
+  archetype = archetype->getDesugaredType();
+  assert(archetype->is<ArchetypeType>() && "The type should be an archetype");
+  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;
+}
+
+/// Check if there are any unresolved forward definitions of opened
+/// archetypes.
+bool SILOpenedArchetypesTracker::hasUnresolvedOpenedArchetypeDefinitions() {
+  for (auto &KV : getOpenedArchetypeDefs()) {
+    assert(KV.getFirst()->is<ArchetypeType>() && "The type should be an archetype");
+    if (!KV.getSecond() || isa<GlobalAddrInst>(KV.getSecond()))
+      return true;
+  }
+  return false;
+}
+
+void SILOpenedArchetypesTracker::registerUsedOpenedArchetypes(Type Ty) {
+  // Nothing else to be done if the type does not contain an opened archetype.
+  if (!Ty || !Ty->hasOpenedExistential())
+    return;
+
+  // Find all opened existentials used by this type and check if their
+  // definitions are known.
+  Ty.visit([&](Type ty) {
+    if (!ty->isOpenedExistential())
+      return;
+
+    ty = ty->getDesugaredType();
+
+    // Nothing to do if a definition was seen already.
+    if (getOpenedArchetypeDef(ty))
+      return;
+
+    auto &SILMod = this->getFunction().getModule();
+    // Create a placeholder representing a forward definition.
+    auto Placeholder = new (SILMod)
+        GlobalAddrInst(SILDebugLocation(), SILMod.Types.getLoweredType(ty));
+    // Make it available to SILBuilder, so that instructions using this
+    // archetype can be constructed.
+    addOpenedArchetypeDef(ty, Placeholder);
+  });
+}
+
 // 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

lib/Serialization/Deserialization.cpp

@@ -4072,6 +4072,9 @@ Type ModuleFile::getType(TypeID TID) {
     return nullptr;
   }
 
+  // Invoke the callback on the deserialized type.
+  DeserializedTypeCallback(typeOrOffset);
+
   return typeOrOffset;
 }
 

lib/Serialization/DeserializeSIL.cpp

@@ -12,6 +12,7 @@
 
 #define DEBUG_TYPE "deserialize"
 #include "DeserializeSIL.h"
+#include "swift/Basic/Defer.h"
 #include "swift/Serialization/ModuleFile.h"
 #include "SILFormat.h"
 #include "swift/SIL/SILArgument.h"
@@ -559,6 +560,16 @@ SILFunction *SILDeserializer::readSILFunction(DeclID FID,
   // is required for adding typedef operands to instructions.
   Builder.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
 
+  // Define a callback to be invoked on the deserialized types.
+  auto OldDeserializedTypeCallback = MF->DeserializedTypeCallback;
+  SWIFT_DEFER {
+    MF->DeserializedTypeCallback = OldDeserializedTypeCallback;
+  };
+
+  MF->DeserializedTypeCallback = [&OpenedArchetypesTracker] (Type ty) {
+    OpenedArchetypesTracker.registerUsedOpenedArchetypes(ty);
+  };
+
   // Another SIL_FUNCTION record means the end of this SILFunction.
   // SIL_VTABLE or SIL_GLOBALVAR or SIL_WITNESS_TABLE record also means the end
   // of this SILFunction.
@@ -598,6 +609,12 @@ SILFunction *SILDeserializer::readSILFunction(DeclID FID,
   if (fn->empty() && errorIfEmptyBody)
     return nullptr;
 
+  // Check that there are no unresolved forward definitions of opened
+  // archetypes.
+  if (OpenedArchetypesTracker.hasUnresolvedOpenedArchetypeDefinitions())
+    llvm_unreachable(
+        "All forward definitions of opened archetypes should be resolved");
+
   if (Callback)
     Callback->didDeserializeFunctionBody(MF->getAssociatedModule(), fn);
 

lib/Serialization/ModuleFile.cpp

@@ -755,7 +755,8 @@ ModuleFile::ModuleFile(
       ModuleInputReader(getStartBytePtr(this->ModuleInputBuffer.get()),
                         getEndBytePtr(this->ModuleInputBuffer.get())),
       ModuleDocInputReader(getStartBytePtr(this->ModuleDocInputBuffer.get()),
-                           getEndBytePtr(this->ModuleDocInputBuffer.get())) {
+                           getEndBytePtr(this->ModuleDocInputBuffer.get())),
+      DeserializedTypeCallback([](Type ty) {}) {
   assert(getStatus() == Status::Valid);
   Bits.IsFramework = isFramework;