[PATCH] D72893: [NewPassManager] Add assertions when getting statefull cached analysis.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: llvm/lib/Analysis/CGSCCPassManager.cpp:224-233
+  // This allows us to rely on the FunctionAnalysisManagerModuleProxy to
   // invalidate the function analyses.
-  auto &MAM = AM.getResult<ModuleAnalysisManagerCGSCCProxy>(C, CG).getManager();
+  auto &MAMProxy =
+      AM.getResult<ModuleAnalysisManagerCGSCCProxy>(C, CG).getInnerManager();
   Module &M = *C.begin()->getFunction().getParent();
-  auto *FAMProxy = MAM.getCachedResult<FunctionAnalysisManagerModuleProxy>(M);
+  auto *FAMProxy =
+      MAMProxy.getCachedResult<FunctionAnalysisManagerModuleProxy>(M);
----------------
So, now that I actually read this code correctly, I understand better why this just doesn't fit anything.

The problem here is that we're not supposed to reconstruct module analyses that *must* be available at this layer. Those are supposed to be passed as parameters to the `run` method the way the `LazyCallGraph` is. Because then, an actual module pass (the CG adapter) will run them (as it can) and pass them along. But that's not a good fit *here* because every other CGSCC pass should use *this* proxy rather than one passed in...

The CGSCC layering is so complicated. I really wish I had any ideas how to really simplify things. Anyways.


I think there is one step that would be slightly cleaner than this. I would change this to just assert that the FAM proxy has been cached, and to construct an *empty* result object.

Then I would change the result object to allow the CGSCC walk (CGSCCPassManager.h:833) to actually get the result, and update it with the FAM. There, the code is actually a *module* pass, and so it can directly get the `FAMProxy` without issue. That will also resolve the FIXME on line 831 in that file as there will be a clear reason why we have to specially set up this proxy -- we need to pass something from the Module layer into the SCC layer that could otherwise be invalidated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72893/new/

https://reviews.llvm.org/D72893