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

[Ellis Hoag via Phabricator via llvm-commits]

ellis added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:1025
+  if (DebugInfoCorrelate) {
+    if (isa<InstrProfCoverInst>(Inc)) {
+      // Mark this global as used so optimizations don't remove it since it is
----------------
kyulee wrote:
> I think we can just delete this condition.
> Without DebugInfoCorrelate, Counters are referenced by Data which is being explicitly held as below via `CompilerUsedVars.push_back(Data);`.
> With DebugInfoCorrelate, now Counters can be optimized away since they are local globals while there are no explicit use.
> I think the reason the Coverage case (write only) does not work while the Counter case (read/write) works is simply because the global opt is smart enough to optimize the Coverage case but not the Counter case -- in theory there is no explicit use in the chain of references on Counters (other than compiler-rt/runtime), and the compiler may optimize them.
> 
Yep, I agree. I have a separate diff D115981 to fix this and will update this code after that lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116180