Skip to content

Commit 4330cef

Browse files
author
Mark Lacey
committed
Remove call graph maintenance from the perf inliner.
This removes the call graph maintenance code from the performance inliner since we've decided to go in a different direction and rather than keeping a persistent call graph are instead splitting it up into a few utilities that can provide the most generally useful information from the call graph. I had to add an unfortunate hack here due to the implicit lazy linking that we have in the compiler, but intend on removing. In theory a change like this should have resulted in no change in fucntionality, but in fact not updating the call graph meant not doing some of that lazy deserialization, which in turn changed what got inlined. This would have been okay, but it seems to have exposed other bugs in the optimizer, so I've added in an explicit call to a function that will deserialize newly devirtualized function references.
1 parent ce86736 commit 4330cef

File tree

1 file changed

+49
-82
lines changed

1 file changed

+49
-82
lines changed

lib/SILPasses/IPO/PerformanceInliner.cpp

Lines changed: 49 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
#include "swift/SIL/SILModule.h"
1717
#include "swift/SIL/Projection.h"
1818
#include "swift/SILAnalysis/BasicCalleeAnalysis.h"
19-
#include "swift/SILAnalysis/CallGraphAnalysis.h"
2019
#include "swift/SILAnalysis/ColdBlockInfo.h"
2120
#include "swift/SILAnalysis/DominanceAnalysis.h"
2221
#include "swift/SILAnalysis/FunctionOrder.h"
@@ -250,22 +249,20 @@ namespace {
250249
/// \p Caller and the inliner needs to reject this inlining request.
251250
bool hasInliningCycle(SILFunction *Caller, SILFunction *Callee);
252251

253-
FullApplySite devirtualizeUpdatingCallGraph(FullApplySite Apply,
254-
CallGraph &CG);
252+
FullApplySite devirtualize(FullApplySite Apply);
255253

256254
bool devirtualizeAndSpecializeApplies(
257255
llvm::SmallVectorImpl<ApplySite> &Applies,
258-
CallGraphAnalysis *CGA,
259256
SILModuleTransform *MT,
260257
llvm::SmallVectorImpl<SILFunction *> &WorkList);
261258

262-
ApplySite specializeGenericUpdatingCallGraph(ApplySite Apply,
263-
CallGraph &CG,
264-
llvm::SmallVectorImpl<ApplySite> &NewApplies);
259+
ApplySite specializeGeneric(ApplySite Apply,
260+
llvm::SmallVectorImpl<ApplySite> &NewApplies);
265261

266-
bool inlineCallsIntoFunction(SILFunction *F, DominanceAnalysis *DA,
267-
SILLoopAnalysis *LA, CallGraph &CG,
268-
llvm::SmallVectorImpl<FullApplySite> &NewApplies);
262+
bool
263+
inlineCallsIntoFunction(SILFunction *F, DominanceAnalysis *DA,
264+
SILLoopAnalysis *LA,
265+
llvm::SmallVectorImpl<FullApplySite> &NewApplies);
269266

270267
public:
271268
SILPerformanceInliner(int threshold,
@@ -275,7 +272,6 @@ namespace {
275272

276273
void inlineDevirtualizeAndSpecialize(SILFunction *WorkItem,
277274
SILModuleTransform *MT,
278-
CallGraphAnalysis *CGA,
279275
DominanceAnalysis *DA,
280276
SILLoopAnalysis *LA);
281277
};
@@ -844,30 +840,38 @@ static bool isProfitableInColdBlock(SILFunction *Callee) {
844840
return true;
845841
}
846842

843+
/// FIXME: Total hack to work around issues exposed while ripping call
844+
/// graph maintenance from the inliner.
845+
static void tryLinkCallee(FullApplySite Apply) {
846+
auto *F = Apply.getCalleeFunction();
847+
if (!F || F->isDefinition()) return;
848+
849+
auto &M = Apply.getFunction()->getModule();
850+
M.linkFunction(F, SILModule::LinkingMode::LinkAll);
851+
}
852+
853+
// Attempt to devirtualize. When successful, replaces the old apply
854+
// with the new one and returns the new one. When unsuccessful returns
855+
// an empty apply site.
856+
FullApplySite SILPerformanceInliner::devirtualize(FullApplySite Apply) {
847857

848-
// Attempt to devirtualize, maintaining the call graph if
849-
// successful. When successful, replaces the old apply with the new
850-
// one and returns the new one. When unsuccessful returns an empty
851-
// apply site.
852-
FullApplySite SILPerformanceInliner::devirtualizeUpdatingCallGraph(
853-
FullApplySite Apply,
854-
CallGraph &CG) {
855858
auto NewInstPair = tryDevirtualizeApply(Apply);
856859
if (!NewInstPair.second)
857860
return FullApplySite();
858861

859-
auto NewAI = FullApplySite::isa(NewInstPair.second.getInstruction());
860-
CallGraphEditor(&CG).replaceApplyWithNew(Apply, NewAI);
861-
862862
replaceDeadApply(Apply, NewInstPair.first);
863+
auto NewApply = FullApplySite(NewInstPair.second.getInstruction());
863864

864-
return NewAI;
865+
// FIXME: This should not be needed. We should instead be linking
866+
// everything in up front, including everything transitively
867+
// referenced through vtables and witness tables.
868+
tryLinkCallee(NewApply);
869+
870+
return FullApplySite(NewInstPair.second.getInstruction());
865871
}
866872

867-
ApplySite SILPerformanceInliner::specializeGenericUpdatingCallGraph(
868-
ApplySite Apply,
869-
CallGraph &CG,
870-
llvm::SmallVectorImpl<ApplySite> &NewApplies) {
873+
ApplySite SILPerformanceInliner::specializeGeneric(
874+
ApplySite Apply, llvm::SmallVectorImpl<ApplySite> &NewApplies) {
871875
assert(NewApplies.empty() && "Expected out parameter for new applies!");
872876

873877
if (!Apply.hasSubstitutions())
@@ -892,15 +896,9 @@ ApplySite SILPerformanceInliner::specializeGenericUpdatingCallGraph(
892896
if (!Specialized)
893897
return ApplySite();
894898

895-
// Add the specialization to the call graph.
896-
CallGraphEditor Editor(&CG);
897-
if (SpecializedFunction)
898-
Editor.addNewFunction(SpecializedFunction);
899-
900899
// Track the new applies from the specialization.
901900
for (auto NewCallSite : Collector.getInstructionPairs())
902-
if (auto NewApply = ApplySite::isa(NewCallSite.first))
903-
NewApplies.push_back(NewApply);
901+
NewApplies.push_back(ApplySite(NewCallSite.first));
904902

905903
auto FullApply = FullApplySite::isa(Apply.getInstruction());
906904

@@ -910,19 +908,14 @@ ApplySite SILPerformanceInliner::specializeGenericUpdatingCallGraph(
910908

911909
// Replace the old apply with the new and delete the old.
912910
replaceDeadApply(Apply, Specialized.getInstruction());
913-
Editor.updatePartialApplyUses(Specialized);
914911

915912
return ApplySite(Specialized);
916913
}
917914

918-
// Update call graph edges
919-
auto SpecializedFullApply = FullApplySite(Specialized.getInstruction());
920-
Editor.replaceApplyWithNew(FullApply, SpecializedFullApply);
921-
922915
// Replace the old apply with the new and delete the old.
923916
replaceDeadApply(Apply, Specialized.getInstruction());
924917

925-
return SpecializedFullApply;
918+
return Specialized;
926919
}
927920

928921
static void collectAllAppliesInFunction(SILFunction *F,
@@ -935,23 +928,21 @@ static void collectAllAppliesInFunction(SILFunction *F,
935928
Applies.push_back(Apply);
936929
}
937930

938-
// Devirtualize and specialize a group of applies, updating the call
939-
// graph and returning a worklist of newly exposed function references
940-
// that should be considered for inlining before continuing with the
941-
// caller that has the passed-in applies.
931+
// Devirtualize and specialize a group of applies, returning a
932+
// worklist of newly exposed function references that should be
933+
// considered for inlining before continuing with the caller that has
934+
// the passed-in applies.
942935
//
943936
// The returned worklist is stacked such that the last things we want
944937
// to process are earlier on the list.
945938
//
946939
// Returns true if any changes were made.
947940
bool SILPerformanceInliner::devirtualizeAndSpecializeApplies(
948941
llvm::SmallVectorImpl<ApplySite> &Applies,
949-
CallGraphAnalysis *CGA,
950942
SILModuleTransform *MT,
951943
llvm::SmallVectorImpl<SILFunction *> &WorkList) {
952944
assert(WorkList.empty() && "Expected empty worklist for return results!");
953945

954-
auto &CG = CGA->getCallGraph();
955946
bool ChangedAny = false;
956947

957948
// The set of all new function references generated by
@@ -966,16 +957,15 @@ bool SILPerformanceInliner::devirtualizeAndSpecializeApplies(
966957

967958
bool ChangedApply = false;
968959
if (auto FullApply = FullApplySite::isa(Apply.getInstruction())) {
969-
if (auto NewApply = devirtualizeUpdatingCallGraph(FullApply, CG)) {
960+
if (auto NewApply = devirtualize(FullApply)) {
970961
ChangedApply = true;
971962

972963
Apply = ApplySite(NewApply.getInstruction());
973964
}
974965
}
975966

976967
llvm::SmallVector<ApplySite, 4> NewApplies;
977-
if (auto NewApply = specializeGenericUpdatingCallGraph(Apply, CG,
978-
NewApplies)) {
968+
if (auto NewApply = specializeGeneric(Apply, NewApplies)) {
979969
ChangedApply = true;
980970

981971
Apply = NewApply;
@@ -995,10 +985,8 @@ bool SILPerformanceInliner::devirtualizeAndSpecializeApplies(
995985

996986
// TODO: Do we need to invalidate everything at this point?
997987
// What about side-effects analysis? What about type analysis?
998-
CGA->lockInvalidation();
999988
MT->invalidateAnalysis(Apply.getFunction(),
1000989
SILAnalysis::InvalidationKind::Everything);
1001-
CGA->unlockInvalidation();
1002990
}
1003991
}
1004992

@@ -1075,7 +1063,6 @@ void SILPerformanceInliner::collectAppliesToInline(
10751063
bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller,
10761064
DominanceAnalysis *DA,
10771065
SILLoopAnalysis *LA,
1078-
CallGraph &CG,
10791066
llvm::SmallVectorImpl<FullApplySite> &NewApplies) {
10801067
// Don't optimize functions that are marked with the opt.never attribute.
10811068
if (!Caller->shouldOptimize())
@@ -1115,11 +1102,10 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller,
11151102
SmallVector<SILValue, 8> Args;
11161103
for (const auto &Arg : AI.getArguments())
11171104
Args.push_back(Arg);
1118-
1119-
// As we inline and clone we need to collect instructions that
1120-
// require updates to the call graph.
1105+
1106+
// As we inline and clone we need to collect new applies.
11211107
auto Filter = [](SILInstruction *I) -> bool {
1122-
return I->mayRelease();
1108+
return bool(FullApplySite::isa(I));
11231109
};
11241110

11251111
CloneCollector Collector(Filter);
@@ -1141,17 +1127,8 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller,
11411127
// we expect it to have happened.
11421128
assert(Success && "Expected inliner to inline this function!");
11431129
llvm::SmallVector<FullApplySite, 4> AppliesFromInlinee;
1144-
llvm::SmallVector<SILInstruction *, 4> NewCallSites;
1145-
for (auto &P : Collector.getInstructionPairs()) {
1146-
NewCallSites.push_back(P.first);
1147-
1148-
if (auto FullApply = FullApplySite::isa(P.first)) {
1149-
AppliesFromInlinee.push_back(FullApply);
1150-
}
1151-
}
1152-
1153-
CallGraphEditor Editor(&CG);
1154-
Editor.replaceApplyWithCallSites(AI, NewCallSites);
1130+
for (auto &P : Collector.getInstructionPairs())
1131+
AppliesFromInlinee.push_back(FullApplySite(P.first));
11551132

11561133
recursivelyDeleteTriviallyDeadInstructions(AI.getInstruction(), true);
11571134

@@ -1175,17 +1152,13 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller,
11751152
void SILPerformanceInliner::inlineDevirtualizeAndSpecialize(
11761153
SILFunction *Caller,
11771154
SILModuleTransform *MT,
1178-
CallGraphAnalysis *CGA,
11791155
DominanceAnalysis *DA,
11801156
SILLoopAnalysis *LA) {
1181-
assert(Caller->isDefinition() &&
1182-
"Expected only defined functions in the call graph!");
1157+
assert(Caller->isDefinition() && "Expected only functions with bodies!");
11831158

11841159
llvm::SmallVector<SILFunction *, 4> WorkList;
11851160
WorkList.push_back(Caller);
11861161

1187-
auto &CG = CGA->getOrBuildCallGraph();
1188-
11891162
while (!WorkList.empty()) {
11901163
llvm::SmallVector<ApplySite, 4> WorkItemApplies;
11911164
SILFunction *CurrentCaller = WorkList.back();
@@ -1196,7 +1169,7 @@ void SILPerformanceInliner::inlineDevirtualizeAndSpecialize(
11961169
// and collect new functions we should inline into as we do
11971170
// so.
11981171
llvm::SmallVector<SILFunction *, 4> NewFuncs;
1199-
if (devirtualizeAndSpecializeApplies(WorkItemApplies, CGA, MT, NewFuncs)) {
1172+
if (devirtualizeAndSpecializeApplies(WorkItemApplies, MT, NewFuncs)) {
12001173
WorkList.insert(WorkList.end(), NewFuncs.begin(), NewFuncs.end());
12011174
NewFuncs.clear();
12021175
}
@@ -1229,23 +1202,19 @@ void SILPerformanceInliner::inlineDevirtualizeAndSpecialize(
12291202
// Inlining in turn might result in new applies that we should
12301203
// consider for devirtualization and specialization.
12311204
llvm::SmallVector<FullApplySite, 4> NewApplies;
1232-
bool Inlined = inlineCallsIntoFunction(WorkItem, DA, LA, CG, NewApplies);
1205+
bool Inlined = inlineCallsIntoFunction(WorkItem, DA, LA, NewApplies);
12331206
if (Inlined) {
1234-
// Invalidate analyses, but lock the call graph since we
1235-
// maintain it.
1236-
CGA->lockInvalidation();
12371207
MT->invalidateAnalysis(WorkItem,
12381208
SILAnalysis::InvalidationKind::FunctionBody);
1239-
CGA->unlockInvalidation();
12401209

12411210
// FIXME: Update inlineCallsIntoFunction to collect all
12421211
// remaining applies after inlining, not just those
12431212
// resulting from inlining code.
12441213
llvm::SmallVector<ApplySite, 4> WorkItemApplies;
12451214
collectAllAppliesInFunction(WorkItem, WorkItemApplies);
12461215

1247-
bool Modified = devirtualizeAndSpecializeApplies(WorkItemApplies, CGA,
1248-
MT, NewFuncs);
1216+
bool Modified =
1217+
devirtualizeAndSpecializeApplies(WorkItemApplies, MT, NewFuncs);
12491218
if (Modified) {
12501219
WorkList.insert(WorkList.end(), NewFuncs.begin(), NewFuncs.end());
12511220
NewFuncs.clear();
@@ -1327,7 +1296,6 @@ class SILPerformanceInlinerPass : public SILModuleTransform {
13271296

13281297
void run() override {
13291298
BasicCalleeAnalysis *BCA = PM->getAnalysis<BasicCalleeAnalysis>();
1330-
CallGraphAnalysis *CGA = PM->getAnalysis<CallGraphAnalysis>();
13311299
DominanceAnalysis *DA = PM->getAnalysis<DominanceAnalysis>();
13321300
SILLoopAnalysis *LA = PM->getAnalysis<SILLoopAnalysis>();
13331301

@@ -1352,8 +1320,7 @@ class SILPerformanceInlinerPass : public SILModuleTransform {
13521320

13531321
// Inline functions bottom up from the leafs.
13541322
while (!WorkList.empty()) {
1355-
Inliner.inlineDevirtualizeAndSpecialize(WorkList.back(), this, CGA, DA,
1356-
LA);
1323+
Inliner.inlineDevirtualizeAndSpecialize(WorkList.back(), this, DA, LA);
13571324
WorkList.pop_back();
13581325
}
13591326
}

0 commit comments

Comments
 (0)