[PATCH] D124490: [InstrProf] Minimal Block Coverage

[Julian Mestre via Phabricator via llvm-commits]

jmestre added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/BlockCoverageInference.cpp:149
+        });
+    if (!HasSuperReachablePred)
+      for (auto *Pred : Preds)
----------------
MaskRay wrote:
> I have other things to do and haven't read very carefully, but the impl here appears to differ from the algorithm description in the summary.
> 
> Having a SuperReachable Pred does not mean that `PredecessorDependencies[&BB]` should remain empty.
As long as BB itself is reachable from EntryBlock, PredecessorDependencies should be non-empty whenever HasSuperReachablePred is true. Consider a simple path (i.e., there are no repeated nodes in the path) from EntryBlock to BB. Let Pred be the second to last node in this path. (Pred is well defined because HasSuperReachablePred is true implies BB != EntryBlock and so the path has at least one edge.) Then Pred belongs in RecheableFromEntry and so it will be added to PredecessorDependencies.
If it is desirable that the implementation handles functions with blocks that are not reachable from EntryBlock, then we can filter them out at the start and set their coverage status of these block to false right way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124490