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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D126586: [I... Gulfem Savrun Yeniceri via Phabricator via cfe-commits
    • [PATCH] D12658... David Li via Phabricator via cfe-commits
    • [PATCH] D12658... Gulfem Savrun Yeniceri via Phabricator via cfe-commits

Reply via email to