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
This commit is contained in:
Erik Eckstein
2021-04-23 14:13:26 +02:00
parent 619226b480
commit eecb9fa975
8 changed files with 118 additions and 28 deletions

View File

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

View File

@@ -304,7 +304,13 @@ private:
/// The value is either a SingleValueInstrution or a PlaceholderValue, in case /// The value is either a SingleValueInstrution or a PlaceholderValue, in case
/// an opened-archetype definition is lookedup during parsing or deserializing /// an opened-archetype definition is lookedup during parsing or deserializing
/// SIL, where opened archetypes can be forward referenced. /// 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. /// The number of PlaceholderValues in openedArchetypeDefs.
int numUnresolvedOpenedArchetypes = 0; int numUnresolvedOpenedArchetypes = 0;
@@ -382,15 +388,18 @@ public:
/// In case the opened archetype is not defined yet (e.g. during parsing or /// In case the opened archetype is not defined yet (e.g. during parsing or
/// deserilization), a PlaceholderValue is returned. This should not be the /// deserilization), a PlaceholderValue is returned. This should not be the
/// case outside of parsing or deserialization. /// 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 /// Returns the instruction which defines an opened archetype, e.g. an
/// open_existential_addr. /// open_existential_addr.
/// ///
/// In contrast to getOpenedArchetypeDef, it is required that all opened /// In contrast to getOpenedArchetypeDef, it is required that all opened
/// archetypes are resolved. /// archetypes are resolved.
SingleValueInstruction *getOpenedArchetypeInst(CanArchetypeType archetype) { SingleValueInstruction *getOpenedArchetypeInst(CanArchetypeType archetype,
return cast<SingleValueInstruction>(getOpenedArchetypeDef(archetype)); SILFunction *inFunction) {
return cast<SingleValueInstruction>(getOpenedArchetypeDef(archetype,
inFunction));
} }
/// Returns true if there are unresolved opened archetypes in the module. /// Returns true if there are unresolved opened archetypes in the module.
@@ -401,6 +410,9 @@ public:
/// Called by SILBuilder whenever a new instruction is created and inserted. /// Called by SILBuilder whenever a new instruction is created and inserted.
void notifyAddedInstruction(SILInstruction *inst); 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. /// Add a delete notification handler \p Handler to the module context.
void registerDeleteNotificationHandler(DeleteNotificationHandler* Handler); void registerDeleteNotificationHandler(DeleteNotificationHandler* Handler);

View File

@@ -383,6 +383,30 @@ SILBasicBlock *SILFunction::createBasicBlockBefore(SILBasicBlock *beforeBB) {
return newBlock; 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) { void SILFunction::moveBlockBefore(SILBasicBlock *BB, SILFunction::iterator IP) {
assert(BB->getParent() == this); assert(BB->getParent() == this);
if (SILFunction::iterator(BB) == IP) if (SILFunction::iterator(BB) == IP)

View File

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

View File

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

View File

@@ -1322,7 +1322,7 @@ public:
"Operand is of an ArchetypeType that does not exist in the " "Operand is of an ArchetypeType that does not exist in the "
"Caller's generic param list."); "Caller's generic param list.");
if (auto OpenedA = getOpenedArchetypeOf(A)) { if (auto OpenedA = getOpenedArchetypeOf(A)) {
auto *openingInst = F->getModule().getOpenedArchetypeInst(OpenedA); auto *openingInst = F->getModule().getOpenedArchetypeInst(OpenedA, F);
require(I == nullptr || openingInst == I || require(I == nullptr || openingInst == I ||
properlyDominates(openingInst, I), properlyDominates(openingInst, I),
"Use of an opened archetype should be dominated by a " "Use of an opened archetype should be dominated by a "
@@ -1456,7 +1456,8 @@ public:
FoundOpenedArchetypes.insert(A); FoundOpenedArchetypes.insert(A);
// Also check that they are properly tracked inside the current // Also check that they are properly tracked inside the current
// function. // function.
auto *openingInst = F.getModule().getOpenedArchetypeInst(A); auto *openingInst = F.getModule().getOpenedArchetypeInst(A,
AI->getFunction());
require(openingInst == AI || require(openingInst == AI ||
properlyDominates(openingInst, AI), properlyDominates(openingInst, AI),
"Use of an opened archetype should be dominated by a " "Use of an opened archetype should be dominated by a "
@@ -3404,7 +3405,8 @@ public:
auto archetype = getOpenedArchetypeOf(OEI->getType().getASTType()); auto archetype = getOpenedArchetypeOf(OEI->getType().getASTType());
require(archetype, require(archetype,
"open_existential_addr result must be an opened existential 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 " "Archetype opened by open_existential_addr should be registered in "
"SILFunction"); "SILFunction");
@@ -3437,7 +3439,8 @@ public:
auto archetype = getOpenedArchetypeOf(resultInstanceTy); auto archetype = getOpenedArchetypeOf(resultInstanceTy);
require(archetype, require(archetype,
"open_existential_ref result must be an opened existential 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 " "Archetype opened by open_existential_ref should be registered in "
"SILFunction"); "SILFunction");
} }
@@ -3459,7 +3462,8 @@ public:
auto archetype = getOpenedArchetypeOf(resultInstanceTy); auto archetype = getOpenedArchetypeOf(resultInstanceTy);
require(archetype, require(archetype,
"open_existential_box result must be an opened existential 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 " "Archetype opened by open_existential_box should be registered in "
"SILFunction"); "SILFunction");
} }
@@ -3481,7 +3485,8 @@ public:
auto archetype = getOpenedArchetypeOf(resultInstanceTy); auto archetype = getOpenedArchetypeOf(resultInstanceTy);
require(archetype, require(archetype,
"open_existential_box_value result not an opened existential 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 " "Archetype opened by open_existential_box_value should be "
"registered in SILFunction"); "registered in SILFunction");
} }
@@ -3527,7 +3532,7 @@ public:
require(archetype, "open_existential_metatype result must be an opened " require(archetype, "open_existential_metatype result must be an opened "
"existential metatype"); "existential metatype");
require( require(
I->getModule().getOpenedArchetypeInst(archetype) == I, I->getModule().getOpenedArchetypeInst(archetype, I->getFunction()) == I,
"Archetype opened by open_existential_metatype should be registered in " "Archetype opened by open_existential_metatype should be registered in "
"SILFunction"); "SILFunction");
} }
@@ -3546,7 +3551,8 @@ public:
auto archetype = getOpenedArchetypeOf(OEI->getType().getASTType()); auto archetype = getOpenedArchetypeOf(OEI->getType().getASTType());
require(archetype, "open_existential_value result must be an opened " require(archetype, "open_existential_value result must be an opened "
"existential archetype"); "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 " "Archetype opened by open_existential should be registered in "
"SILFunction"); "SILFunction");
} }
@@ -3834,7 +3840,7 @@ public:
SILValue Def; SILValue Def;
if (t->isOpenedExistential()) { if (t->isOpenedExistential()) {
auto archetypeTy = cast<ArchetypeType>(t); 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"); require(Def, "Opened archetype should be registered in SILModule");
} else if (t->hasDynamicSelfType()) { } else if (t->hasDynamicSelfType()) {
require(I->getFunction()->hasSelfParam() || require(I->getFunction()->hasSelfParam() ||

View File

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

View File

@@ -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)
}