[PATCH] D124490: [InstrProf] Minimal Block Coverage

[Ellis Hoag via Phabricator via llvm-commits]

ellis added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/BlockCoverageInference.cpp:221
+  // If we have a choice to infer coverage from a block's predecessor or
+  // successor, choose its predecessor.
+  for (auto &BB : *F)
----------------
spupyrev wrote:
> why predecessor? is this for determinism?
The predecessor was just an arbitrary choice.

On second thought, this logic is not really needed. The original idea was to have every non-instrumented node depend on either some of its predecessors or some of its successors. This aligns well with the theory, but in practice we don't think this is needed. This may result in a node's coverage depending on more nodes than necessary, but this should be fine. I will remove this logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124490