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