[PATCH] D116180: [InstrProf] Add single byte coverage mode

Mon Jun 6 11:19:53 PDT 2022

ellis added a comment.

In D116180#3558453 <https://reviews.llvm.org/D116180#3558453>, @MaskRay wrote:

> In D116180#3276998 <https://reviews.llvm.org/D116180#3276998>, @kyulee wrote:
>
>> LGTM. I'm approving this but suggest for other reviewers'. Also I left a few comments about naming counters that may follow up.
>
> Thanks for contributing a new coverage mode. I believe it will be useful to some groups.
> That said, process-wise, I think committing this just 6hr after this comment was probably too quick. 6hr did not give stakeholders enough time to read through a major new feature.
> Some stakeholders may have other commitments so don't respond timely. But if you gave more time with the patch in an "approved" state, they would probably want to comment.
>
> I think my main question is why does a coverage mode need to piggyback on `PGOInstrument.cpp` which is mainly for optimizations.
>
> From the functionality implemented by `-pgo-function-entry-coverage`, I don't see how it can be used for optimization.

I apologize for moving too quickly to land. In the future, I'll try to allow more time for others to review and respect their time.

As for your question, I've posted this RFC [0] describing our plans for lightweight instrumentation. Basically, we were planning to create a few instrumentation modes that trade off profile precision for a more lightweight instrumented binary. 1) block coverage, 2) function entry count, and 3) function entry coverage. As you've seen, D124490 <https://reviews.llvm.org/D124490> implements block coverage and the profiles are useful for optimization in D125743 <https://reviews.llvm.org/D125743> which uses block liveness to guide the machine outliner.

Function entry coverage implemented in this diff was an important step towards block coverage instrumentation. Both modes share a lot of code, which is a big reason why I added this in `PGOInstrumentation.cpp`. Unlike block coverage instrumentation, function entry coverage is not precise enough to properly guide optimizations, so its main use is to detect dead code.

[0] https://groups.google.com/g/llvm-dev/c/r03Z6JoN7d4

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116180