llvm.org GIT mirror llvm / 143ef32
[PM] Finish implementing and fix a chain of bugs uncovered by testing the invalidation propagation logic from an SCC to a Function. I wrote the infrastructure to test this but didn't actually use it in the unit test where it was designed to be used. =[ My bad. Once I actually added it to the test case I discovered that it also hadn't been properly implemented, so I've implemented it. The logic in the FAM proxy for an SCC pass to propagate invalidation follows the same ideas as the FAM proxy for a Module pass, but the implementation is a bit different to reflect the fact that it is forwarding just for an SCC. However, implementing this correctly uncovered a surprising "bug" (it was conservatively correct but relatively very expensive) in how we handle invalidation when splitting one SCC into multiple SCCs. We did an eager invalidation when in reality we should be deferring invaliadtion for the *current* SCC to the CGSCC pass manager and just invaliating the newly constructed SCCs. Otherwise we end up invalidating too much too soon. This was exposed by the inliner test case that I've updated. Now, we invalidate *just* the split off '(test1_f)' SCC when doing the CG update, and then the inliner finishes and invalidates the '(test1_g, test1_h)' SCC's analyses. The first few attempts at fixing this hit still more bugs, but all of those are covered by existing tests. For example, the inliner should also preserve the FAM proxy to avoid unnecesasry invalidation, and this is safe because the CG update routines it uses handle any necessary adjustments to the FAM proxy. Finally, the unittests for the CGSCC pass manager needed a bunch of updates where we weren't correctly preserving the FAM proxy because it hadn't been fully implemented and failing to preserve it didn't matter. Note that this doesn't yet fix the current crasher due to MemSSA finding a stale dominator tree, but without this the fix to that crasher doesn't really make any sense when testing because it relies on the proxy behavior. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@307487 91177308-0d34-0410-b5e6-96231b3b80d8 Chandler Carruth 3 years ago
4 changed file(s) with 147 addition(s) and 37 deletion(s). Raw diff Collapse all Expand all
195195 bool FunctionAnalysisManagerCGSCCProxy::Result::invalidate(
196196 LazyCallGraph::SCC &C, const PreservedAnalyses &PA,
197197 CGSCCAnalysisManager::Invalidator &Inv) {
198 for (LazyCallGraph::Node &N : C)
199 FAM->invalidate(N.getFunction(), PA);
200
201 // This proxy doesn't need to handle invalidation itself. Instead, the
202 // module-level CGSCC proxy handles it above by ensuring that if the
203 // module-level FAM proxy becomes invalid the entire SCC layer, which
204 // includes this proxy, is cleared.
198 // If literally everything is preserved, we're done.
199 if (PA.areAllPreserved())
200 return false; // This is still a valid proxy.
201
202 // If this proxy isn't marked as preserved, then even if the result remains
203 // valid, the key itself may no longer be valid, so we clear everything.
204 //
205 // Note that in order to preserve this proxy, a module pass must ensure that
206 // the FAM has been completely updated to handle the deletion of functions.
207 // Specifically, any FAM-cached results for those functions need to have been
208 // forcibly cleared. When preserved, this proxy will only invalidate results
209 // cached on functions *still in the module* at the end of the module pass.
210 auto PAC = PA.getChecker();
211 if (!PAC.preserved() && !PAC.preservedSet>()) {
212 for (LazyCallGraph::Node &N : C)
213 FAM->clear(N.getFunction());
214
215 return true;
216 }
217
218 // Directly check if the relevant set is preserved.
219 bool AreFunctionAnalysesPreserved =
220 PA.allAnalysesInSetPreserved>();
221
222 // Now walk all the functions to see if any inner analysis invalidation is
223 // necessary.
224 for (LazyCallGraph::Node &N : C) {
225 Function &F = N.getFunction();
226 Optional FunctionPA;
227
228 // Check to see whether the preserved set needs to be pruned based on
229 // SCC-level analysis invalidation that triggers deferred invalidation
230 // registered with the outer analysis manager proxy for this function.
231 if (auto *OuterProxy =
232 FAM->getCachedResult(F))
233 for (const auto &OuterInvalidationPair :
234 OuterProxy->getOuterInvalidations()) {
235 AnalysisKey *OuterAnalysisID = OuterInvalidationPair.first;
236 const auto &InnerAnalysisIDs = OuterInvalidationPair.second;
237 if (Inv.invalidate(OuterAnalysisID, C, PA)) {
238 if (!FunctionPA)
239 FunctionPA = PA;
240 for (AnalysisKey *InnerAnalysisID : InnerAnalysisIDs)
241 FunctionPA->abandon(InnerAnalysisID);
242 }
243 }
244
245 // Check if we needed a custom PA set, and if so we'll need to run the
246 // inner invalidation.
247 if (FunctionPA) {
248 FAM->invalidate(F, *FunctionPA);
249 continue;
250 }
251
252 // Otherwise we only need to do invalidation if the original PA set didn't
253 // preserve all function analyses.
254 if (!AreFunctionAnalysesPreserved)
255 FAM->invalidate(F, PA);
256 }
257
258 // Return false to indicate that this result is still a valid proxy.
205259 return false;
206260 }
207261
235289 dbgs() << "Enqueuing the existing SCC in the worklist:" << *C << "\n";
236290
237291 SCC *OldC = C;
238 (void)OldC;
239292
240293 // Update the current SCC. Note that if we have new SCCs, this must actually
241294 // change the SCC.
243296 "Cannot insert new SCCs without changing current SCC!");
244297 C = &*NewSCCRange.begin();
245298 assert(G.lookupSCC(N) == C && "Failed to update current SCC!");
299
300 // If we had a cached FAM proxy originally, we will want to create more of
301 // them for each SCC that was split off.
302 bool NeedFAMProxy =
303 AM.getCachedResult(*OldC) != nullptr;
304
305 // We need to propagate an invalidation call to all but the newly current SCC
306 // because the outer pass manager won't do that for us after splitting them.
307 // FIXME: We should accept a PreservedAnalysis from the CG updater so that if
308 // there are preserved ananalyses we can avoid invalidating them here for
309 // split-off SCCs.
310 // We know however that this will preserve any FAM proxy so go ahead and mark
311 // that.
312 PreservedAnalyses PA;
313 PA.preserve();
314 AM.invalidate(*OldC, PA);
315
316 // Ensure we have a proxy for the now-current SCC if needed.
317 if (NeedFAMProxy)
318 (void)AM.getResult(*C, G);
246319
247320 for (SCC &NewC :
248321 reverse(make_range(std::next(NewSCCRange.begin()), NewSCCRange.end()))) {
251324 UR.CWorklist.insert(&NewC);
252325 if (DebugLogging)
253326 dbgs() << "Enqueuing a newly formed SCC:" << NewC << "\n";
327
328 // Ensure new SCCs have a FAM proxy if needed.
329 if (NeedFAMProxy)
330 (void)AM.getResult(*C, G);
331
332 // And propagate an invalidation to the new SCC as only the current will
333 // get one from the pass manager infrastructure.
334 AM.invalidate(NewC, PA);
254335 }
255336 return C;
256337 }
348429 // For separate SCCs this is trivial.
349430 RC->switchTrivialInternalEdgeToRef(N, TargetN);
350431 } else {
351 // Otherwise we may end up re-structuring the call graph. First,
352 // invalidate any SCC analyses. We have to do this before we split
353 // functions into new SCCs and lose track of where their analyses are
354 // cached.
355 // FIXME: We should accept a more precise preserved set here. For
356 // example, it might be possible to preserve some function analyses
357 // even as the SCC structure is changed.
358 AM.invalidate(*C, PreservedAnalyses::none());
359432 // Now update the call graph.
360433 C = incorporateNewSCCRange(RC->switchInternalEdgeToRef(N, TargetN), G,
361434 N, C, AM, UR, DebugLogging);
423496 continue;
424497 }
425498
426 // Otherwise we may end up re-structuring the call graph. First, invalidate
427 // any SCC analyses. We have to do this before we split functions into new
428 // SCCs and lose track of where their analyses are cached.
429 // FIXME: We should accept a more precise preserved set here. For example,
430 // it might be possible to preserve some function analyses even as the SCC
431 // structure is changed.
432 AM.invalidate(*C, PreservedAnalyses::none());
433499 // Now update the call graph.
434500 C = incorporateNewSCCRange(RC->switchInternalEdgeToRef(N, *RefTarget), G, N,
435501 C, AM, UR, DebugLogging);
988988 // And delete the actual function from the module.
989989 M.getFunctionList().erase(DeadF);
990990 }
991 return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
992 }
991
992 if (!Changed)
993 return PreservedAnalyses::all();
994
995 // Even if we change the IR, we update the core CGSCC data structures and so
996 // can preserve the proxy to the function analysis manager.
997 PreservedAnalyses PA;
998 PA.preserve();
999 return PA;
1000 }
1010 ; CHECK: Running analysis: FunctionAnalysisManagerCGSCCProxy on (test1_f, test1_g, test1_h)
1111 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
1212 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
13 ; CHECK: Invalidating all non-preserved analyses for: (test1_f, test1_g, test1_h)
13 ; CHECK: Invalidating all non-preserved analyses for: (test1_f)
1414 ; CHECK: Invalidating all non-preserved analyses for: test1_f
1515 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
16 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
17 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
18 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
19 ; CHECK: Invalidating all non-preserved analyses for: (test1_g, test1_h)
1620 ; CHECK: Invalidating all non-preserved analyses for: test1_g
1721 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
18 ; CHECK: Invalidating all non-preserved analyses for: test1_h
19 ; CHECK-NOT: Invalidating anaylsis:
20 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_h
21 ; CHECK: Invalidating all non-preserved analyses for: (test1_g, test1_h)
22 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
23 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
24 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_g
2225 ; CHECK: Invalidating all non-preserved analyses for: test1_h
2326 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_h
27 ; CHECK: Invalidating analysis: LoopAnalysis on test1_h
28 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_h
29 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_h
30 ; CHECK-NOT: Invalidating analysis:
31 ; CHECK: Starting llvm::Function pass manager run.
32 ; CHECK-NEXT: Running pass: DominatorTreeVerifierPass on test1_g
33 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_g
34 ; CHECK-NEXT: Finished llvm::Function pass manager run.
35 ; CHECK-NEXT: Starting llvm::Function pass manager run.
36 ; CHECK-NEXT: Running pass: DominatorTreeVerifierPass on test1_h
37 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
38 ; CHECK-NEXT: Finished llvm::Function pass manager run.
39 ; CHECK-NOT: Invalidating analysis:
40 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
41 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
2442
2543 ; An external function used to control branches.
2644 declare i1 @flag()
679679 LazyCallGraph &, CGSCCUpdateResult &) {
680680 PreservedAnalyses PA;
681681 PA.preserve();
682 PA.preserve();
682683 PA.preserve();
683684 return PA;
684685 }));
718719 CGPM1.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM1)));
719720 MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM1)));
720721
721 // Now run a module pass that preserves the LazyCallGraph and proxy but not
722 // Now run a module pass that preserves the LazyCallGraph and proxies but not
722723 // the Function analysis.
723724 MPM.addPass(LambdaModulePass([&](Module &M, ModuleAnalysisManager &) {
724725 PreservedAnalyses PA;
725726 PA.preserve();
726727 PA.preserve();
728 PA.preserve();
729 PA.preserve();
727730 return PA;
728731 }));
729732
740743 EXPECT_EQ(2 * 6, FunctionAnalysisRuns);
741744 }
742745
743 // Check that by marking the function pass and FAM proxy as preserved, this
746 // Check that by marking the function pass and proxies as preserved, this
744747 // propagates all the way through.
745748 TEST_F(CGSCCPassManagerTest,
746749 TestModulePassCanPreserveFunctionAnalysisNestedInCGSCC) {
764767 PreservedAnalyses PA;
765768 PA.preserve();
766769 PA.preserve();
770 PA.preserve();
767771 PA.preserve();
768772 PA.preserve();
769773 return PA;
10131017 FunctionCount += IndirectResult.SCCDep.FunctionCount;
10141018 return PreservedAnalyses::all();
10151019 }));
1020 CGPM.addPass(createCGSCCToFunctionPassAdaptor(
1021 RequireAnalysisPass()));
1022
10161023 // Next, invalidate
10171024 // - both analyses for the (f) and (x) SCCs,
10181025 // - just the underlying (indirect) analysis for (g) SCC, and
10251032 auto &IndirectResult = DoublyIndirectResult.IDep;
10261033 FunctionCount += IndirectResult.SCCDep.FunctionCount;
10271034 auto PA = PreservedAnalyses::none();
1035 PA.preserve();
1036 PA.preserveSet>();
10281037 if (C.getName() == "(g)")
10291038 PA.preserve();
10301039 else if (C.getName() == "(h3, h1, h2)")
10311040 PA.preserve();
10321041 return PA;
10331042 }));
1034 // Finally, use the analysis again on each function, forcing re-computation
1035 // for all of them.
1043 // Finally, use the analysis again on each SCC (and function), forcing
1044 // re-computation for all of them.
10361045 CGPM.addPass(
10371046 LambdaSCCPass([&](LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
10381047 LazyCallGraph &CG, CGSCCUpdateResult &) {
10421051 FunctionCount += IndirectResult.SCCDep.FunctionCount;
10431052 return PreservedAnalyses::all();
10441053 }));
1054 CGPM.addPass(createCGSCCToFunctionPassAdaptor(
1055 RequireAnalysisPass()));
10451056
10461057 // Create a second CGSCC pass manager. This will cause the module-level
10471058 // invalidation to occur, which will force yet another invalidation of the
10571068 FunctionCount += IndirectResult.SCCDep.FunctionCount;
10581069 return PreservedAnalyses::all();
10591070 }));
1060
1061 // Add a requires pass to populate the module analysis and then our function
1071 CGPM2.addPass(createCGSCCToFunctionPassAdaptor(
1072 RequireAnalysisPass()));
1073
1074 // Add a requires pass to populate the module analysis and then our CGSCC
10621075 // pass pipeline.
10631076 MPM.addPass(RequireAnalysisPass());
10641077 MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
10651078 // Now require the module analysis again (it will have been invalidated once)
1066 // and then use it again from a function pass manager.
1079 // and then use it again from our second CGSCC pipeline..
10671080 MPM.addPass(RequireAnalysisPass());
10681081 MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM2)));
10691082 MPM.run(*M, MAM);
10791092 EXPECT_EQ(3 * 4, IndirectSCCAnalysisRuns);
10801093 EXPECT_EQ(3 * 4, DoublyIndirectSCCAnalysisRuns);
10811094
1095 // We run the indirect function analysis once per function the first time.
1096 // Then we re-run it for every SCC but "(g)". Then we re-run it for every
1097 // function again.
1098 EXPECT_EQ(6 + 5 + 6, IndirectFunctionAnalysisRuns);
1099
10821100 // Four passes count each of six functions once (via SCCs).
10831101 EXPECT_EQ(4 * 6, FunctionCount);
10841102 }