[PATCH] D124490: [InstrProf] Minimal Block Coverage

[Sergey Pupyrev via Phabricator via llvm-commits]

spupyrev accepted this revision.
spupyrev added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Enna1.

We reviewed the code internally prior sending upstream, and it is still looks good to me.



================
Comment at: llvm/lib/Transforms/Instrumentation/BlockCoverageInference.cpp:179
+      // This is the first node on the path, return its neighbor.
+      assert(Neighbors.size() == 1);
+      return Neighbors.front();
----------------
I have seen in some places assertions of type
```
assert(condition && "readable text message");
```
Is this optional or the accepted style?


================
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)
----------------
why predecessor? is this for determinism?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124490