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

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/InstrProf.h:289
+  SingleByteCoverage = 0x10, // Use single byte coverage probes.
+  FunctionEntryOnly = 0x20,  // Only instrument the function entry.
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/FunctionEntryOnly)
----------------
ellis wrote:
> snehasish wrote:
> > At a glance, this looks similar to the `BB` enum value. If this is coverage only can we make it explicit in the name? Perhaps something like `FunctionEntryCoverageOnly`. IIUC the comment implies "only instrument the function entry for coverage" as opposed to whether the the entry basic block has instrumentation for profiling.
> > 
> > I like the idea of having better naming for the enum values and I'll send out a separate patch renaming the others once this is submitted.
> This is actually not for coverage. `FunctionEntryOnly` only instruments the entry basic block, whereas BB guarantees that the entry basic block is instrumented, along with other blocks.
I see. So should we allow profiles with BB and FunctionEntryOnly to be merged? If not can you update the logic in mergeProfileKind in InstrProfWriter.h?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116180