[PATCH] D78987: [BPI][NFC] Reuse post dominantor tree from analysis manager when available

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

Hi Alina,
to me the patch seems to be NFC.

Before the patch, BranchProbabilityInfo::calculate built PDT unconditionally and this was not reflected in pipeline (that is the reason test for pipeline changed).

If I understand correctly patch does the following two modifications:

1. in BranchProbabilityInfo::calculate, if PDT is provided -> use it. Otherwise unconditionally build it.
2. In usage of BranchProbabilityInfo::calculate, Analysis manager is requested for PDT and it is provided as argument to BranchProbabilityInfo::calculate.

Your proposed change is actually, request the cached analysis and if it does not exist BranchProbabilityInfo::calculate will build it anyway.

So in both cases (Yevgeny's patch and your proposal) PDT will be built anyway, the only question when.
In Yevgeny's patch it will be built as analysis. In your proposal, it will be built inside BranchProbabilityInfo::calculate.

To me there is no difference.

Do I miss anything?



================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:1011
 
-  std::unique_ptr<PostDominatorTree> PDT =
-      std::make_unique<PostDominatorTree>(const_cast<Function &>(F));
----------------
Unconditionally build PDT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78987