[PATCH] D126586: [InstrProf][WIP] Implement boolean counters in coverage

[Gulfem Savrun Yeniceri via Phabricator via llvm-commits]

gulfem added a comment.

In D126586#3646197 <https://reviews.llvm.org/D126586#3646197>, @davidxl wrote:

> For coverage mode, why using 'incrementProfileCounter'?  Should it be set to '1' instead?  Also is the 'or' expression needed? The expression can be folded to either zero or 1.

@davidxl I used `incrementProfileCounter()`, but it emits `llvm.instrprof.cover` intrinsic instead of `llvm.instrprof.increment` in `emitCounterIncrement()`. If it confusing, I can move the boolean counter handling into another function and name it like `setProfileCounter`, `setProfileCovered` or something like that.

I have one concern that I would like to hear your recommendation: `incrementProfileCounter()` is embedded into `Clang CodeGen` for various `AST` nodes. So, even for implementing a simple prototype to collect some numbers will require me doing invasive changes to `Clang CodeGen`. These are going to be small changes to several AST nodes. I have only done it for some frequently used `AST` nodes, but there are many more of them. Can you think of a better way of designing that? Based on the current implementation of instrumentation, I could not find a less invasive way of implementing boolean counters. If that's the only way to go, shall we land them in stages after the reviews?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126586