gulfem marked an inline comment as done.
gulfem added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:1080-1081
+void CodeGenPGO::setProfileVersion(llvm::Module &M) {
+  if (CGM.getCodeGenOpts().hasProfileClangInstr() &&
+      llvm::EnableSingleByteCoverage) {
+    const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
----------------
zequanwu wrote:
> I thinkit's better to emit the profile version global variable in 
> `CoverageMappingModuleGen::emit` so this check 
> `CGM.getCodeGenOpts().hasProfileClangInstr()` is not necessary. 
> 
> Also we should always emit the profile version global variable so that the 
> runtime/backend knows if single byte coverage is enabled or not by looking at 
> the version.
`CoverageMappingModuleGen` is only invoked when `-fcoverage-mapping` flag is 
provided, and single byte counters are emitted when `-fprofile-instr-generate` 
flag is provided. We provide both flags for coverage purposes, but if we move 
this global to `CoverageMappingModuleGen::emit`, single byte counters won't 
work if `-fcoverage-mapping` is not provided. I don't know whether there will 
be such use cases, but for this reason it might not be a good idea to put that 
into `CoverageMappingModuleGen`.


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... Gulfem Savrun Yeniceri via Phabricator via cfe-commits

Reply via email to