Skip to content

[DO NOT MERGE] Revert "SILOptimizer: Fix analysis invalidation after devirtualization." #30513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
atrick wants to merge 1 commit into from
Closed

[DO NOT MERGE] Revert "SILOptimizer: Fix analysis invalidation after devirtualization." #30513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions include/swift/SILOptimizer/Utils/Devirtualize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,9 @@ SubstitutionMap getWitnessMethodSubstitutions(SILModule &Module, ApplySite AI,
///
/// If this succeeds, the caller must call deleteDevirtualizedApply on
/// the original apply site.
///
/// Return the new apply and true if the CFG was also modified.
std::pair<ApplySite, bool>
tryDevirtualizeApply(ApplySite AI, ClassHierarchyAnalysis *CHA,
OptRemark::Emitter *ORE = nullptr);
ApplySite tryDevirtualizeApply(ApplySite AI,
ClassHierarchyAnalysis *CHA,
OptRemark::Emitter *ORE = nullptr);
bool canDevirtualizeApply(FullApplySite AI, ClassHierarchyAnalysis *CHA);
bool canDevirtualizeClassMethod(FullApplySite AI, ClassDecl *CD,
OptRemark::Emitter *ORE = nullptr,
Expand All @@ -81,23 +79,21 @@ CanType getSelfInstanceType(CanType ClassOrMetatypeType);
/// Devirtualize the given apply site, which is known to be devirtualizable.
///
/// The caller must call deleteDevirtualizedApply on the original apply site.
///
/// Return the new apply and true if the CFG was also modified.
std::pair<FullApplySite, bool> devirtualizeClassMethod(FullApplySite AI,
SILValue ClassInstance,
ClassDecl *CD,
OptRemark::Emitter *ORE);
FullApplySite devirtualizeClassMethod(FullApplySite AI,
SILValue ClassInstance,
ClassDecl *CD,
OptRemark::Emitter *ORE);

/// Attempt to devirtualize the given apply site, which is known to be
/// of a class method. If this fails, the returned FullApplySite will be null.
///
/// If this succeeds, the caller must call deleteDevirtualizedApply on
/// the original apply site.
///
/// Return the new apply and true if the CFG was also modified.
std::pair<FullApplySite, bool>
tryDevirtualizeClassMethod(FullApplySite AI, SILValue ClassInstance,
ClassDecl *CD, OptRemark::Emitter *ORE,
FullApplySite
tryDevirtualizeClassMethod(FullApplySite AI,
SILValue ClassInstance,
ClassDecl *CD,
OptRemark::Emitter *ORE,
bool isEffectivelyFinalMethod = false);

/// Attempt to devirtualize the given apply site, which is known to be
Expand All @@ -106,9 +102,7 @@ tryDevirtualizeClassMethod(FullApplySite AI, SILValue ClassInstance,
///
/// If this succeeds, the caller must call deleteDevirtualizedApply on
/// the original apply site.
///
/// Return the new apply and true if the CFG was also modified.
std::pair<ApplySite, bool> tryDevirtualizeWitnessMethod(ApplySite AI, OptRemark::Emitter *ORE);
ApplySite tryDevirtualizeWitnessMethod(ApplySite AI, OptRemark::Emitter *ORE);

/// Delete a successfully-devirtualized apply site. This must always be
/// called after devirtualizing an apply; not only is it not semantically
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Mandatory/MandatoryInlining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ getCalleeFunction(SILFunction *F, FullApplySite AI, bool &IsThick,

static SILInstruction *tryDevirtualizeApplyHelper(FullApplySite InnerAI,
ClassHierarchyAnalysis *CHA) {
auto NewInst = tryDevirtualizeApply(InnerAI, CHA).first;
auto NewInst = tryDevirtualizeApply(InnerAI, CHA);
if (!NewInst)
return InnerAI.getInstruction();

Expand Down
22 changes: 7 additions & 15 deletions lib/SILOptimizer/Transforms/Devirtualizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ using namespace swift;
namespace {

class Devirtualizer : public SILFunctionTransform {
bool Changed = false;
bool ChangedCFG = false;

void devirtualizeAppliesInFunction(SILFunction &F,
bool devirtualizeAppliesInFunction(SILFunction &F,
ClassHierarchyAnalysis *CHA);

/// The entry point to the transformation.
Expand All @@ -43,22 +41,17 @@ class Devirtualizer : public SILFunctionTransform {
LLVM_DEBUG(llvm::dbgs() << "***** Devirtualizer on function:" << F.getName()
<< " *****\n");

Changed = false;
ChangedCFG = false;
devirtualizeAppliesInFunction(F, CHA);
if (ChangedCFG)
invalidateAnalysis(SILAnalysis::InvalidationKind::Everything);
else if (Changed)
if (devirtualizeAppliesInFunction(F, CHA))
invalidateAnalysis(SILAnalysis::InvalidationKind::CallsAndInstructions);
}

};

} // end anonymous namespace

// Return true if any calls changed, and true if the CFG also changed.
void Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F,
bool Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F,
ClassHierarchyAnalysis *CHA) {
bool Changed = false;
llvm::SmallVector<ApplySite, 8> NewApplies;
OptRemark::Emitter ORE(DEBUG_TYPE, F.getModule());

Expand All @@ -76,14 +69,11 @@ void Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F,
}
}
for (auto Apply : Applies) {
ApplySite NewInst;
bool modifiedCFG;
std::tie(NewInst, modifiedCFG) = tryDevirtualizeApply(Apply, CHA, &ORE);
auto NewInst = tryDevirtualizeApply(Apply, CHA, &ORE);
if (!NewInst)
continue;

Changed = true;
ChangedCFG |= modifiedCFG;

deleteDevirtualizedApply(Apply);
NewApplies.push_back(NewInst);
Expand Down Expand Up @@ -115,6 +105,8 @@ void Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F,
if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize())
addFunctionToPassManagerWorklist(CalleeFn, nullptr);
}

return Changed;
}

SILTransform *swift::createDevirtualizer() { return new Devirtualizer(); }
9 changes: 3 additions & 6 deletions lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@ static FullApplySite speculateMonomorphicTarget(FullApplySite AI,

// Devirtualize the apply instruction on the identical path.
auto NewInst =
devirtualizeClassMethod(IdenAI, DownCastedClassInstance, CD, nullptr)
.first;
devirtualizeClassMethod(IdenAI, DownCastedClassInstance, CD, nullptr);
assert(NewInst && "Expected to be able to devirtualize apply!");
(void)NewInst;

Expand Down Expand Up @@ -415,8 +414,7 @@ static bool tryToSpeculateTarget(FullApplySite AI, ClassHierarchyAnalysis *CHA,
// try to devirtualize it completely.
ClassHierarchyAnalysis::ClassList Subs;
if (isDefaultCaseKnown(CHA, AI, CD, Subs)) {
auto NewInst =
tryDevirtualizeClassMethod(AI, SubTypeValue, CD, &ORE).first;
auto NewInst = tryDevirtualizeClassMethod(AI, SubTypeValue, CD, &ORE);
if (NewInst)
deleteDevirtualizedApply(AI);
return bool(NewInst);
Expand Down Expand Up @@ -576,8 +574,7 @@ static bool tryToSpeculateTarget(FullApplySite AI, ClassHierarchyAnalysis *CHA,
ORE.emit(RB);
return true;
}
auto NewInst =
tryDevirtualizeClassMethod(AI, SubTypeValue, CD, nullptr).first;
auto NewInst = tryDevirtualizeClassMethod(AI, SubTypeValue, CD, nullptr);
if (NewInst) {
ORE.emit(RB);
deleteDevirtualizedApply(AI);
Expand Down
103 changes: 45 additions & 58 deletions lib/SILOptimizer/Utils/Devirtualize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,12 +482,12 @@ static ApplyInst *replaceApplyInst(SILBuilder &builder, SILLocation loc,
return newAI;
}

// Return the new try_apply and true if a cast required CFG modification.
static std::pair<TryApplyInst *, bool>
replaceTryApplyInst(SILBuilder &builder, SILLocation loc, TryApplyInst *oldTAI,
SILValue newFn, SubstitutionMap newSubs,
ArrayRef<SILValue> newArgs, SILFunctionConventions conv,
ArrayRef<SILValue> newArgBorrows) {
static TryApplyInst *replaceTryApplyInst(SILBuilder &builder, SILLocation loc,
TryApplyInst *oldTAI, SILValue newFn,
SubstitutionMap newSubs,
ArrayRef<SILValue> newArgs,
SILFunctionConventions conv,
ArrayRef<SILValue> newArgBorrows) {
SILBasicBlock *normalBB = oldTAI->getNormalBB();
SILBasicBlock *resultBB = nullptr;

Expand Down Expand Up @@ -537,7 +537,7 @@ replaceTryApplyInst(SILBuilder &builder, SILLocation loc, TryApplyInst *oldTAI,
}

builder.setInsertionPoint(normalBB->begin());
return {newTAI, resultCastRequired};
return newTAI;
}

static BeginApplyInst *
Expand Down Expand Up @@ -599,18 +599,17 @@ replacePartialApplyInst(SILBuilder &builder, SILLocation loc,
return newPAI;
}

// Return the new apply and true if the CFG was also modified.
static std::pair<ApplySite, bool>
replaceApplySite(SILBuilder &builder, SILLocation loc, ApplySite oldAS,
SILValue newFn, SubstitutionMap newSubs,
ArrayRef<SILValue> newArgs, SILFunctionConventions conv,
ArrayRef<SILValue> newArgBorrows) {
static ApplySite replaceApplySite(SILBuilder &builder, SILLocation loc,
ApplySite oldAS, SILValue newFn,
SubstitutionMap newSubs,
ArrayRef<SILValue> newArgs,
SILFunctionConventions conv,
ArrayRef<SILValue> newArgBorrows) {
switch (oldAS.getKind()) {
case ApplySiteKind::ApplyInst: {
auto *oldAI = cast<ApplyInst>(oldAS);
return {replaceApplyInst(builder, loc, oldAI, newFn, newSubs, newArgs,
newArgBorrows),
false};
return replaceApplyInst(builder, loc, oldAI, newFn, newSubs, newArgs,
newArgBorrows);
}
case ApplySiteKind::TryApplyInst: {
auto *oldTAI = cast<TryApplyInst>(oldAS);
Expand All @@ -619,16 +618,14 @@ replaceApplySite(SILBuilder &builder, SILLocation loc, ApplySite oldAS,
}
case ApplySiteKind::BeginApplyInst: {
auto *oldBAI = dyn_cast<BeginApplyInst>(oldAS);
return {replaceBeginApplyInst(builder, loc, oldBAI, newFn, newSubs, newArgs,
newArgBorrows),
false};
return replaceBeginApplyInst(builder, loc, oldBAI, newFn, newSubs, newArgs,
newArgBorrows);
}
case ApplySiteKind::PartialApplyInst: {
assert(newArgBorrows.empty());
auto *oldPAI = cast<PartialApplyInst>(oldAS);
return {
replacePartialApplyInst(builder, loc, oldPAI, newFn, newSubs, newArgs),
false};
return replacePartialApplyInst(builder, loc, oldPAI, newFn, newSubs,
newArgs);
}
}
llvm_unreachable("covered switch");
Expand Down Expand Up @@ -732,12 +729,10 @@ bool swift::canDevirtualizeClassMethod(FullApplySite applySite, ClassDecl *cd,
/// \p ClassOrMetatype is a class value or metatype value that is the
/// self argument of the apply we will devirtualize.
/// return the result value of the new ApplyInst if created one or null.
///
/// Return the new apply and true if the CFG was also modified.
std::pair<FullApplySite, bool>
swift::devirtualizeClassMethod(FullApplySite applySite,
SILValue classOrMetatype, ClassDecl *cd,
OptRemark::Emitter *ore) {
FullApplySite swift::devirtualizeClassMethod(FullApplySite applySite,
SILValue classOrMetatype,
ClassDecl *cd,
OptRemark::Emitter *ore) {
LLVM_DEBUG(llvm::dbgs() << " Trying to devirtualize : "
<< *applySite.getInstruction());

Expand Down Expand Up @@ -798,10 +793,8 @@ swift::devirtualizeClassMethod(FullApplySite applySite,
newArgs.push_back(arg);
++paramArgIter;
}
ApplySite newAS;
bool modifiedCFG;
std::tie(newAS, modifiedCFG) = replaceApplySite(
builder, loc, applySite, fri, subs, newArgs, substConv, newArgBorrows);
ApplySite newAS = replaceApplySite(builder, loc, applySite, fri, subs,
newArgs, substConv, newArgBorrows);
FullApplySite newAI = FullApplySite::isa(newAS.getInstruction());
assert(newAI);

Expand All @@ -815,14 +808,16 @@ swift::devirtualizeClassMethod(FullApplySite applySite,
});
NumClassDevirt++;

return {newAI, modifiedCFG};
return newAI;
}

std::pair<FullApplySite, bool> swift::tryDevirtualizeClassMethod(
FullApplySite applySite, SILValue classInstance, ClassDecl *cd,
OptRemark::Emitter *ore, bool isEffectivelyFinalMethod) {
FullApplySite swift::tryDevirtualizeClassMethod(FullApplySite applySite,
SILValue classInstance,
ClassDecl *cd,
OptRemark::Emitter *ore,
bool isEffectivelyFinalMethod) {
if (!canDevirtualizeClassMethod(applySite, cd, ore, isEffectivelyFinalMethod))
return {FullApplySite(), false};
return FullApplySite();
return devirtualizeClassMethod(applySite, classInstance, cd, ore);
}

Expand Down Expand Up @@ -965,12 +960,9 @@ swift::getWitnessMethodSubstitutions(SILModule &module, ApplySite applySite,
/// Generate a new apply of a function_ref to replace an apply of a
/// witness_method when we've determined the actual function we'll end
/// up calling.
///
/// Return the new apply and true if the CFG was also modified.
static std::pair<ApplySite, bool>
devirtualizeWitnessMethod(ApplySite applySite, SILFunction *f,
ProtocolConformanceRef cRef,
OptRemark::Emitter *ore) {
static ApplySite devirtualizeWitnessMethod(ApplySite applySite, SILFunction *f,
ProtocolConformanceRef cRef,
OptRemark::Emitter *ore) {
// We know the witness thunk and the corresponding set of substitutions
// required to invoke the protocol method at this point.
auto &module = applySite.getModule();
Expand Down Expand Up @@ -1025,9 +1017,7 @@ devirtualizeWitnessMethod(ApplySite applySite, SILFunction *f,
SILLocation loc = applySite.getLoc();
auto *fri = applyBuilder.createFunctionRefFor(loc, f);

ApplySite newApplySite;
bool modifiedCFG;
std::tie(newApplySite, modifiedCFG) =
ApplySite newApplySite =
replaceApplySite(applyBuilder, loc, applySite, fri, subMap, arguments,
substConv, borrowedArgs);

Expand All @@ -1039,7 +1029,7 @@ devirtualizeWitnessMethod(ApplySite applySite, SILFunction *f,
<< "Devirtualized call to " << NV("Method", f);
});
NumWitnessDevirt++;
return {newApplySite, modifiedCFG};
return newApplySite;
}

static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
Expand Down Expand Up @@ -1076,11 +1066,10 @@ static bool canDevirtualizeWitnessMethod(ApplySite applySite) {
/// In the cases where we can statically determine the function that
/// we'll call to, replace an apply of a witness_method with an apply
/// of a function_ref, returning the new apply.
std::pair<ApplySite, bool>
swift::tryDevirtualizeWitnessMethod(ApplySite applySite,
OptRemark::Emitter *ore) {
ApplySite swift::tryDevirtualizeWitnessMethod(ApplySite applySite,
OptRemark::Emitter *ore) {
if (!canDevirtualizeWitnessMethod(applySite))
return {ApplySite(), false};
return ApplySite();

SILFunction *f;
SILWitnessTable *wt;
Expand All @@ -1099,11 +1088,9 @@ swift::tryDevirtualizeWitnessMethod(ApplySite applySite,

/// Attempt to devirtualize the given apply if possible, and return a
/// new instruction in that case, or nullptr otherwise.
///
/// Return the new apply and true if the CFG was also modified.
std::pair<ApplySite, bool>
swift::tryDevirtualizeApply(ApplySite applySite, ClassHierarchyAnalysis *cha,
OptRemark::Emitter *ore) {
ApplySite swift::tryDevirtualizeApply(ApplySite applySite,
ClassHierarchyAnalysis *cha,
OptRemark::Emitter *ore) {
LLVM_DEBUG(llvm::dbgs() << " Trying to devirtualize: "
<< *applySite.getInstruction());

Expand All @@ -1118,7 +1105,7 @@ swift::tryDevirtualizeApply(ApplySite applySite, ClassHierarchyAnalysis *cha,
// TODO: check if we can also de-virtualize partial applies of class methods.
FullApplySite fas = FullApplySite::isa(applySite.getInstruction());
if (!fas)
return {ApplySite(), false};
return ApplySite();

/// Optimize a class_method and alloc_ref pair into a direct function
/// reference:
Expand Down Expand Up @@ -1164,7 +1151,7 @@ swift::tryDevirtualizeApply(ApplySite applySite, ClassHierarchyAnalysis *cha,
return tryDevirtualizeClassMethod(fas, instance, cd, ore);
}

return {ApplySite(), false};
return ApplySite();
}

bool swift::canDevirtualizeApply(FullApplySite applySite,
Expand Down