[PATCH] D138846: MC/DC in LLVM Source-Based Code Coverage: LLVM back-end and compiler-rt

Tue Jul 18 02:41:36 PDT 2023

chapuni added a comment.

Thank you for working. Could you reformat a few lines?

I know some inconsistencies in `*.inc` but I think we may follow traditional format there.

================
Comment at: llvm/include/llvm/ProfileData/InstrProfData.inc:661
 /* Raw profile format version (start from 1). */
-#define INSTR_PROF_RAW_VERSION 8
+#define INSTR_PROF_RAW_VERSION 9
 /* Indexed profile format version (start from 1). */
----------------
I guess you have forgot regenerating more a few input files.

* `Transforms/PGOProfile/memprof.ll`

================
Comment at: llvm/lib/ProfileData/InstrProfCorrelator.cpp:196-198
+void InstrProfCorrelatorImpl<IntPtrT>::addProbe(
+    StringRef FunctionName, uint64_t CFGHash, IntPtrT CounterOffset,
+    IntPtrT FunctionPtr, uint32_t NumCounters) {
----------------
This could be reformatted.

================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:748
+        Twine("Runtime counter relocation is presently not supported for "
+            "MC/DC bitmaps."),
+        DS_Warning));
----------------
Could you reformat?

================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:1058-1060
+  StringRef GroupName = TT.isOSBinFormatCOFF() && DataReferencedByCode
+                            ? GV->getName()
+                            : VarName;
----------------
This could be reformatted.

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

https://reviews.llvm.org/D138846