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

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea 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
----------------
bjope wrote:
> 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.
Good point. I think this should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151666