SILModule: track opened archetypes per function.

In theory we could map opened archetypes per module because opened archetypes _should_ be unique across the module.
But currently in some rare cases SILGen re-uses the same opened archetype in multiple functions.
The fix is to add the SILFunction to the map's key.
That also requires that we update the map whenever instructions are moved from one function to another.

This fixes a compiler crash.

rdar://76916931

Author: eeckstein

include/swift/SIL/SILFunction.h

@@ -1020,19 +1020,12 @@ class SILFunction
 
   /// Transfer all blocks of \p F into this function, at the begin of the block
   /// list.
-  void moveAllBlocksFromOtherFunction(SILFunction *F) {
-    BlockList.splice(begin(), F->BlockList);
-  }
+  void moveAllBlocksFromOtherFunction(SILFunction *F);
 
   /// Transfer \p blockInOtherFunction of another function into this function,
   /// before \p insertPointInThisFunction.
   void moveBlockFromOtherFunction(SILBasicBlock *blockInOtherFunction,
-                                  iterator insertPointInThisFunction) {
-    SILFunction *otherFunc = blockInOtherFunction->getParent();
-    assert(otherFunc != this);
-    BlockList.splice(insertPointInThisFunction, otherFunc->BlockList,
-                     blockInOtherFunction);
-  }
+                                  iterator insertPointInThisFunction);
 
   /// Move block \p BB to immediately before the iterator \p IP.
   ///

include/swift/SIL/SILModule.h

@@ -304,7 +304,13 @@ class SILModule {
   /// The value is either a SingleValueInstrution or a PlaceholderValue, in case
   /// an opened-archetype definition is lookedup during parsing or deserializing
   /// SIL, where opened archetypes can be forward referenced.
-  llvm::DenseMap<ArchetypeType*, SILValue> openedArchetypeDefs;
+  ///
+  /// In theory we wouldn't need to have the SILFunction in the key, because
+  /// opened archetypes _should_ be unique across the module. But currently
+  /// in some rare cases SILGen re-uses the same opened archetype for multiple
+  /// functions.
+  using OpenedArchetypeKey = std::pair<ArchetypeType*, SILFunction*>;
+  llvm::DenseMap<OpenedArchetypeKey, SILValue> openedArchetypeDefs;
 
   /// The number of PlaceholderValues in openedArchetypeDefs.
   int numUnresolvedOpenedArchetypes = 0;
@@ -382,15 +388,18 @@ class SILModule {
   /// In case the opened archetype is not defined yet (e.g. during parsing or
   /// deserilization), a PlaceholderValue is returned. This should not be the
   /// case outside of parsing or deserialization.
-  SILValue getOpenedArchetypeDef(CanArchetypeType archetype);
+  SILValue getOpenedArchetypeDef(CanArchetypeType archetype,
+                                 SILFunction *inFunction);
 
   /// Returns the instruction which defines an opened archetype, e.g. an
   /// open_existential_addr.
   ///
   /// In contrast to getOpenedArchetypeDef, it is required that all opened
   /// archetypes are resolved.
-  SingleValueInstruction *getOpenedArchetypeInst(CanArchetypeType archetype) {
-    return cast<SingleValueInstruction>(getOpenedArchetypeDef(archetype));
+  SingleValueInstruction *getOpenedArchetypeInst(CanArchetypeType archetype,
+                                                 SILFunction *inFunction) {
+    return cast<SingleValueInstruction>(getOpenedArchetypeDef(archetype,
+                                                              inFunction));
   }
 
   /// Returns true if there are unresolved opened archetypes in the module.
@@ -401,6 +410,9 @@ class SILModule {
   /// Called by SILBuilder whenever a new instruction is created and inserted.
   void notifyAddedInstruction(SILInstruction *inst);
 
+  /// Called after an instruction is moved from one function to another.
+  void notifyMovedInstruction(SILInstruction *inst, SILFunction *fromFunction);
+
   /// Add a delete notification handler \p Handler to the module context.
   void registerDeleteNotificationHandler(DeleteNotificationHandler* Handler);
 

lib/SIL/IR/SILFunction.cpp

@@ -383,6 +383,30 @@ SILBasicBlock *SILFunction::createBasicBlockBefore(SILBasicBlock *beforeBB) {
   return newBlock;
 }
 
+void SILFunction::moveAllBlocksFromOtherFunction(SILFunction *F) {
+  BlockList.splice(begin(), F->BlockList);
+  
+  SILModule &mod = getModule();
+  for (SILBasicBlock &block : *this) {
+    for (SILInstruction &inst : block) {
+      mod.notifyMovedInstruction(&inst, F);
+    }
+  }
+}
+
+void SILFunction::moveBlockFromOtherFunction(SILBasicBlock *blockInOtherFunction,
+                                iterator insertPointInThisFunction) {
+  SILFunction *otherFunc = blockInOtherFunction->getParent();
+  assert(otherFunc != this);
+  BlockList.splice(insertPointInThisFunction, otherFunc->BlockList,
+                   blockInOtherFunction);
+
+  SILModule &mod = getModule();
+  for (SILInstruction &inst : *blockInOtherFunction) {
+    mod.notifyMovedInstruction(&inst, otherFunc);
+  }
+}
+
 void SILFunction::moveBlockBefore(SILBasicBlock *BB, SILFunction::iterator IP) {
   assert(BB->getParent() == this);
   if (SILFunction::iterator(BB) == IP)

lib/SIL/IR/SILInstructions.cpp

@@ -76,7 +76,7 @@ static void buildTypeDependentOperands(
     SmallVectorImpl<SILValue> &TypeDependentOperands, SILFunction &F) {
 
   for (auto archetype : OpenedArchetypes) {
-    SILValue def = F.getModule().getOpenedArchetypeDef(archetype);
+    SILValue def = F.getModule().getOpenedArchetypeDef(archetype, &F);
     assert(def->getFunction() == &F &&
            "def of opened archetype is in wrong function");
     TypeDependentOperands.push_back(def);

lib/SIL/IR/SILModule.cpp

@@ -724,8 +724,9 @@ void SILModule::registerDeserializationNotificationHandler(
   deserializationNotificationHandlers.add(std::move(handler));
 }
 
-SILValue SILModule::getOpenedArchetypeDef(CanArchetypeType archetype) {
-  SILValue &def = openedArchetypeDefs[archetype];
+SILValue SILModule::getOpenedArchetypeDef(CanArchetypeType archetype,
+                                          SILFunction *inFunction) {
+  SILValue &def = openedArchetypeDefs[{archetype, inFunction}];
   if (!def) {
     numUnresolvedOpenedArchetypes++;
     def = ::new PlaceholderValue(SILType::getPrimitiveAddressType(archetype));
@@ -741,7 +742,7 @@ bool SILModule::hasUnresolvedOpenedArchetypeDefinitions() {
 void SILModule::notifyAddedInstruction(SILInstruction *inst) {
   if (auto *svi = dyn_cast<SingleValueInstruction>(inst)) {
     if (CanArchetypeType archeTy = svi->getOpenedArchetype()) {
-      SILValue &val = openedArchetypeDefs[archeTy];
+      SILValue &val = openedArchetypeDefs[{archeTy, inst->getFunction()}];
       if (val) {
         if (!isa<PlaceholderValue>(val)) {
           // Print a useful error message (and not just abort with an assert).
@@ -765,6 +766,19 @@ void SILModule::notifyAddedInstruction(SILInstruction *inst) {
   }
 }
 
+void SILModule::notifyMovedInstruction(SILInstruction *inst,
+                                       SILFunction *fromFunction) {
+  if (auto *svi = dyn_cast<SingleValueInstruction>(inst)) {
+    if (CanArchetypeType archeTy = svi->getOpenedArchetype()) {
+      OpenedArchetypeKey key = {archeTy, fromFunction};
+      assert(openedArchetypeDefs.lookup(key) == svi &&
+             "archetype def was not registered");
+      openedArchetypeDefs.erase(key);
+      openedArchetypeDefs[{archeTy, svi->getFunction()}] = svi;
+    }
+  }
+}
+
 void SILModule::registerDeleteNotificationHandler(
     DeleteNotificationHandler *handler) {
   // Ask the handler (that can be an analysis, a pass, or some other data
@@ -783,9 +797,10 @@ void SILModule::notifyDeleteHandlers(SILNode *node) {
   // Update openedArchetypeDefs.
   if (auto *svi = dyn_cast<SingleValueInstruction>(node)) {
     if (CanArchetypeType archeTy = svi->getOpenedArchetype()) {
-      assert(openedArchetypeDefs.lookup(archeTy) == svi &&
+      OpenedArchetypeKey key = {archeTy, svi->getFunction()};
+      assert(openedArchetypeDefs.lookup(key) == svi &&
              "archetype def was not registered");
-      openedArchetypeDefs.erase(archeTy);
+      openedArchetypeDefs.erase(key);
     }
   }
 

lib/SIL/Verifier/SILVerifier.cpp

@@ -1329,7 +1329,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               "Operand is of an ArchetypeType that does not exist in the "
               "Caller's generic param list.");
       if (auto OpenedA = getOpenedArchetypeOf(A)) {
-        auto *openingInst = F->getModule().getOpenedArchetypeInst(OpenedA);
+        auto *openingInst = F->getModule().getOpenedArchetypeInst(OpenedA, F);
         require(I == nullptr || openingInst == I ||
                 properlyDominates(openingInst, I),
                 "Use of an opened archetype should be dominated by a "
@@ -1463,7 +1463,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
         FoundOpenedArchetypes.insert(A);
         // Also check that they are properly tracked inside the current
         // function.
-        auto *openingInst = F.getModule().getOpenedArchetypeInst(A);
+        auto *openingInst = F.getModule().getOpenedArchetypeInst(A,
+                              AI->getFunction());
         require(openingInst == AI ||
                 properlyDominates(openingInst, AI),
                 "Use of an opened archetype should be dominated by a "
@@ -3411,7 +3412,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     auto archetype = getOpenedArchetypeOf(OEI->getType().getASTType());
     require(archetype,
         "open_existential_addr result must be an opened existential archetype");
-    require(OEI->getModule().getOpenedArchetypeInst(archetype) == OEI,
+    require(OEI->getModule().getOpenedArchetypeInst(archetype,
+                 OEI->getFunction()) == OEI,
             "Archetype opened by open_existential_addr should be registered in "
             "SILFunction");
 
@@ -3444,7 +3446,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     auto archetype = getOpenedArchetypeOf(resultInstanceTy);
     require(archetype,
         "open_existential_ref result must be an opened existential archetype");
-    require(OEI->getModule().getOpenedArchetypeInst(archetype) == OEI,
+    require(OEI->getModule().getOpenedArchetypeInst(archetype,
+                 OEI->getFunction()) == OEI,
             "Archetype opened by open_existential_ref should be registered in "
             "SILFunction");
   }
@@ -3466,7 +3469,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     auto archetype = getOpenedArchetypeOf(resultInstanceTy);
     require(archetype,
         "open_existential_box result must be an opened existential archetype");
-    require(OEI->getModule().getOpenedArchetypeInst(archetype) == OEI,
+    require(OEI->getModule().getOpenedArchetypeInst(archetype,
+                 OEI->getFunction()) == OEI,
             "Archetype opened by open_existential_box should be registered in "
             "SILFunction");
   }
@@ -3488,7 +3492,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     auto archetype = getOpenedArchetypeOf(resultInstanceTy);
     require(archetype,
         "open_existential_box_value result not an opened existential archetype");
-    require(OEI->getModule().getOpenedArchetypeInst(archetype) == OEI,
+    require(OEI->getModule().getOpenedArchetypeInst(archetype,
+                 OEI->getFunction()) == OEI,
             "Archetype opened by open_existential_box_value should be "
             "registered in SILFunction");
   }
@@ -3534,7 +3539,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     require(archetype, "open_existential_metatype result must be an opened "
                        "existential metatype");
     require(
-        I->getModule().getOpenedArchetypeInst(archetype) == I,
+        I->getModule().getOpenedArchetypeInst(archetype, I->getFunction()) == I,
         "Archetype opened by open_existential_metatype should be registered in "
         "SILFunction");
   }
@@ -3553,7 +3558,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     auto archetype = getOpenedArchetypeOf(OEI->getType().getASTType());
     require(archetype, "open_existential_value result must be an opened "
                        "existential archetype");
-    require(OEI->getModule().getOpenedArchetypeInst(archetype) == OEI,
+    require(OEI->getModule().getOpenedArchetypeInst(archetype,
+                 OEI->getFunction()) == OEI,
             "Archetype opened by open_existential should be registered in "
             "SILFunction");
   }
@@ -3841,7 +3847,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       SILValue Def;
       if (t->isOpenedExistential()) {
         auto archetypeTy = cast<ArchetypeType>(t);
-        Def = I->getModule().getOpenedArchetypeInst(archetypeTy);
+        Def = I->getModule().getOpenedArchetypeInst(archetypeTy, I->getFunction());
         require(Def, "Opened archetype should be registered in SILModule");
       } else if (t->hasDynamicSelfType()) {
         require(I->getFunction()->hasSelfParam() ||

test/SILGen/Inputs/duplicate_opened_archetypes.h

@@ -0,0 +1,7 @@
+
+@protocol P
+@end
+
+@interface I
+@property(class, readonly) I<P> *x;
+@end

test/SILGen/duplicate_opened_archetypes.swift

@@ -0,0 +1,33 @@
+// RUN: %target-swift-frontend %s -enable-objc-interop -import-objc-header %S/Inputs/duplicate_opened_archetypes.h -emit-sil -o /dev/null
+
+// REQUIRES: objc_interop
+
+// Check that SILGen does not crash because of duplicate opened archetypes
+// in two functions.
+
+import Foundation
+
+struct S {
+    let i: I
+}
+
+@propertyWrapper
+public struct W<Value> {
+    public var wrappedValue: Value
+
+    public init(wrappedValue: Value) {
+        self.wrappedValue = wrappedValue
+    }
+
+    public init(initialValue: Value) {
+        wrappedValue = initialValue
+    }
+
+}
+
+
+struct Test {
+  @W private var s = S(i: .x)
+
+  var t: S = S(i: .x)
+}