[PATCH] D116180: [InstrProf] Add single byte coverage mode
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 4 12:16:47 PDT 2022
MaskRay added a comment.
Herald added subscribers: Enna1, StephenFan.
Herald added a project: All.
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116180/new/
https://reviews.llvm.org/D116180
More information about the llvm-commits
mailing list