[PATCH] D151666: [IPSCCP] Update Post Dominator Tree if Block Frequency Analysis has run.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SCCP.cpp:227
 
-    DomTreeUpdater DTU = IsFuncSpecEnabled && Specializer.isClonedFunction(&F)
-        ? DomTreeUpdater(DomTreeUpdater::UpdateStrategy::Lazy)
-        : Solver.getDTU(F);
-
+    DomTreeUpdater DTU = Specializer.getDTU(F);
     // Change dead blocks to unreachable. We do it after replacing constants
----------------
Had been nice if one could just do something like this here:

```
DominatorTree *DT = FAM->getCachedResult<DominatorTreeAnalysis>(F);
PostDominatorTree *PDT = FAM->getCachedResult<PostDominatorTreeAnalysis>(F);
DomTreeUpdated DTU(DT, PDT, DomTreeUpdater::UpdateStrategy::Lazy};
```

I.e. just use getCachedResult to see if there is a DT/PDT analysis available. If so, we need to update corresponding analyses. If they aren't available we shouldn't run the analysis here.

I think that would work if getCachedResult actually picks up an analysis that is created during the pass execution (any reason why it wouldn't do that?).

Just saying that the PassManager/AnalysisManager should know if PDT has been calculated (so having a map in FunctionSpecialization that keep track of if BlockFrequencyAnalysis has been run seems a bit off, as what we want to know is if the DominatarTree/PostDominiatorTree needs to be updated).

So a solution like this would be more future proof (if it is working) as it wouldn't need to know which other analyses that might end up calculating the PDT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151666