release/19.x: [MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode (#96126) #101629

llvmbot · 2024-08-02T07:59:58Z

Backport d2f77eb 891d898 20d723e

Requested by: @chapuni

…6126) `counter_bias` is incompatible to Bitmap. The distance between Counters and Bitmap is different between on-memory sections and profraw image. Reference to `__llvm_profile_bitmap_bias` is generated only if `-fcoverge-mcdc` `-runtime-counter-relocation` are specified. The current implementation rejected their options. ``` Runtime counter relocation is presently not supported for MC/DC bitmaps ``` (cherry picked from commit d2f77eb)

(cherry picked from commit 891d898)

…ed targets. (cherry picked from commit 20d723e)

llvmbot · 2024-08-02T08:00:04Z

@chapuni What do you think about merging this PR to the release branch?

llvmbot · 2024-08-02T08:00:32Z

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport d2f77eb 891d898 20d723e

Requested by: @chapuni

Full diff: https://github.com/llvm/llvm-project/pull/101629.diff

6 Files Affected:

(modified) compiler-rt/include/profile/InstrProfData.inc (+1)
(modified) compiler-rt/lib/profile/InstrProfilingFile.c (+39-3)
(modified) llvm/include/llvm/ProfileData/InstrProf.h (+4)
(modified) llvm/include/llvm/ProfileData/InstrProfData.inc (+1)
(modified) llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp (+15-11)
(modified) llvm/test/Instrumentation/InstrProfiling/mcdc.ll (+9-3)

diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index 847e53cfa74328..b9df3266fbcf8f 100644
--- a/compiler-rt/include/profile/InstrProfData.inc
+++ b/compiler-rt/include/profile/InstrProfData.inc
@@ -738,6 +738,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
 #define INSTR_PROF_PROFILE_COUNTER_BIAS_VAR __llvm_profile_counter_bias
+#define INSTR_PROF_PROFILE_BITMAP_BIAS_VAR __llvm_profile_bitmap_bias
 #define INSTR_PROF_PROFILE_SET_TIMESTAMP __llvm_profile_set_timestamp
 #define INSTR_PROF_PROFILE_SAMPLING_VAR __llvm_profile_sampling
 
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 1c58584d2d4f73..db3918d8410319 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -101,6 +101,8 @@ static const int UseBiasVar = 0;
 static const char *FileOpenMode = "a+b";
 static void *BiasAddr = NULL;
 static void *BiasDefaultAddr = NULL;
+static void *BitmapBiasAddr = NULL;
+static void *BitmapBiasDefaultAddr = NULL;
 static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
   /* Get the sizes of various profile data sections. Taken from
    * __llvm_profile_get_size_for_buffer(). */
@@ -199,11 +201,15 @@ static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
 #define INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR                            \
   INSTR_PROF_CONCAT(INSTR_PROF_PROFILE_COUNTER_BIAS_VAR, _default)
 COMPILER_RT_VISIBILITY intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR = 0;
+#define INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR                             \
+  INSTR_PROF_CONCAT(INSTR_PROF_PROFILE_BITMAP_BIAS_VAR, _default)
+COMPILER_RT_VISIBILITY intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR = 0;
 
 /* This variable is a weak external reference which could be used to detect
  * whether or not the compiler defined this symbol. */
 #if defined(_MSC_VER)
 COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
+COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR;
 #if defined(_M_IX86) || defined(__i386__)
 #define WIN_SYM_PREFIX "_"
 #else
@@ -213,10 +219,17 @@ COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
     linker, "/alternatename:" WIN_SYM_PREFIX INSTR_PROF_QUOTE(                 \
                 INSTR_PROF_PROFILE_COUNTER_BIAS_VAR) "=" WIN_SYM_PREFIX        \
                 INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR))
+#pragma comment(                                                               \
+    linker, "/alternatename:" WIN_SYM_PREFIX INSTR_PROF_QUOTE(                 \
+                INSTR_PROF_PROFILE_BITMAP_BIAS_VAR) "=" WIN_SYM_PREFIX         \
+                INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR))
 #else
 COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR
     __attribute__((weak, alias(INSTR_PROF_QUOTE(
                              INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR))));
+COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_BITMAP_BIAS_VAR
+    __attribute__((weak, alias(INSTR_PROF_QUOTE(
+                             INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR))));
 #endif
 static const int ContinuousModeSupported = 1;
 static const int UseBiasVar = 1;
@@ -227,6 +240,9 @@ static const char *FileOpenMode = "w+b";
  * used and runtime provides a weak alias so we can check if it's defined. */
 static void *BiasAddr = &INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
 static void *BiasDefaultAddr = &INSTR_PROF_PROFILE_COUNTER_BIAS_DEFAULT_VAR;
+static void *BitmapBiasAddr = &INSTR_PROF_PROFILE_BITMAP_BIAS_VAR;
+static void *BitmapBiasDefaultAddr =
+    &INSTR_PROF_PROFILE_BITMAP_BIAS_DEFAULT_VAR;
 static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
   /* Get the sizes of various profile data sections. Taken from
    * __llvm_profile_get_size_for_buffer(). */
@@ -237,12 +253,18 @@ static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
   const char *BitmapBegin = __llvm_profile_begin_bitmap();
   const char *BitmapEnd = __llvm_profile_end_bitmap();
   uint64_t DataSize = __llvm_profile_get_data_size(DataBegin, DataEnd);
+  uint64_t CountersSize =
+      __llvm_profile_get_counters_size(CountersBegin, CountersEnd);
+  uint64_t NumBitmapBytes =
+      __llvm_profile_get_num_bitmap_bytes(BitmapBegin, BitmapEnd);
   /* Get the file size. */
   uint64_t FileSize = 0;
   if (getProfileFileSizeForMerging(File, &FileSize))
     return 1;
 
   int Fileno = fileno(File);
+  uint64_t PaddingBytesAfterCounters =
+      __llvm_profile_get_num_padding_bytes(CountersSize);
   uint64_t FileOffsetToCounters =
       sizeof(__llvm_profile_header) + __llvm_write_binary_ids(NULL) + DataSize;
 
@@ -260,7 +282,17 @@ static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
   /* Return the memory allocated for counters to OS. */
   lprofReleaseMemoryPagesToOS((uintptr_t)CountersBegin, (uintptr_t)CountersEnd);
 
-  /* BIAS MODE not supported yet for Bitmap (MCDC). */
+  /* Also mmap MCDC bitmap bytes. If there aren't any bitmap bytes, mmap()
+   * will fail with EINVAL. */
+  if (NumBitmapBytes == 0)
+    return 0;
+
+  /* Update profbm_bias. */
+  uint64_t FileOffsetToBitmap =
+      FileOffsetToCounters + CountersSize + PaddingBytesAfterCounters;
+  /* Update the profile fields based on the current mapping. */
+  INSTR_PROF_PROFILE_BITMAP_BIAS_VAR =
+      (uintptr_t)Profile - (uintptr_t)BitmapBegin + FileOffsetToBitmap;
 
   /* Return the memory allocated for counters to OS. */
   lprofReleaseMemoryPagesToOS((uintptr_t)BitmapBegin, (uintptr_t)BitmapEnd);
@@ -272,6 +304,8 @@ static const int UseBiasVar = 0;
 static const char *FileOpenMode = "a+b";
 static void *BiasAddr = NULL;
 static void *BiasDefaultAddr = NULL;
+static void *BitmapBiasAddr = NULL;
+static void *BitmapBiasDefaultAddr = NULL;
 static int mmapForContinuousMode(uint64_t CurrentFileOffset, FILE *File) {
   return 0;
 }
@@ -619,8 +653,10 @@ static void initializeProfileForContinuousMode(void) {
     PROF_ERR("%s\n", "continuous mode is unsupported on this platform");
     return;
   }
-  if (UseBiasVar && BiasAddr == BiasDefaultAddr) {
-    PROF_ERR("%s\n", "__llvm_profile_counter_bias is undefined");
+  if (UseBiasVar && BiasAddr == BiasDefaultAddr &&
+      BitmapBiasAddr == BitmapBiasDefaultAddr) {
+    PROF_ERR("%s\n", "Neither __llvm_profile_counter_bias nor "
+                     "__llvm_profile_bitmap_bias is defined");
     return;
   }
 
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index d449091078c93b..824dcf2372c832 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -174,6 +174,10 @@ inline StringRef getInstrProfCounterBiasVarName() {
   return INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_COUNTER_BIAS_VAR);
 }
 
+inline StringRef getInstrProfBitmapBiasVarName() {
+  return INSTR_PROF_QUOTE(INSTR_PROF_PROFILE_BITMAP_BIAS_VAR);
+}
+
 /// Return the marker used to separate PGO names during serialization.
 inline StringRef getInstrProfNameSeparator() { return "\01"; }
 
diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
index 847e53cfa74328..b9df3266fbcf8f 100644
--- a/llvm/include/llvm/ProfileData/InstrProfData.inc
+++ b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -738,6 +738,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 #define INSTR_PROF_RAW_VERSION_VAR __llvm_profile_raw_version
 #define INSTR_PROF_PROFILE_RUNTIME_VAR __llvm_profile_runtime
 #define INSTR_PROF_PROFILE_COUNTER_BIAS_VAR __llvm_profile_counter_bias
+#define INSTR_PROF_PROFILE_BITMAP_BIAS_VAR __llvm_profile_bitmap_bias
 #define INSTR_PROF_PROFILE_SET_TIMESTAMP __llvm_profile_set_timestamp
 #define INSTR_PROF_PROFILE_SAMPLING_VAR __llvm_profile_sampling
 
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index d1396071c0bade..1805ea89272ec7 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1192,18 +1192,22 @@ CallInst *InstrLowerer::getRMWOrCall(Value *Addr, Value *Val) {
 
 Value *InstrLowerer::getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I) {
   auto *Bitmaps = getOrCreateRegionBitmaps(I);
-  IRBuilder<> Builder(I);
-
-  if (isRuntimeCounterRelocationEnabled()) {
-    LLVMContext &Ctx = M.getContext();
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        M.getName().data(),
-        Twine("Runtime counter relocation is presently not supported for MC/DC "
-              "bitmaps."),
-        DS_Warning));
-  }
+  if (!isRuntimeCounterRelocationEnabled())
+    return Bitmaps;
 
-  return Bitmaps;
+  // Put BiasLI onto the entry block.
+  Type *Int64Ty = Type::getInt64Ty(M.getContext());
+  Function *Fn = I->getFunction();
+  IRBuilder<> EntryBuilder(&Fn->getEntryBlock().front());
+  auto *Bias = getOrCreateBiasVar(getInstrProfBitmapBiasVarName());
+  auto *BiasLI = EntryBuilder.CreateLoad(Int64Ty, Bias, "profbm_bias");
+  // Assume BiasLI invariant (in the function at least)
+  BiasLI->setMetadata(LLVMContext::MD_invariant_load,
+                      MDNode::get(M.getContext(), std::nullopt));
+
+  // Add Bias to Bitmaps and put it before the intrinsic.
+  IRBuilder<> Builder(I);
+  return Builder.CreatePtrAdd(Bitmaps, BiasLI, "profbm_addr");
 }
 
 void InstrLowerer::lowerCover(InstrProfCoverInst *CoverInstruction) {
diff --git a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
index 20c002e0565dbd..fbdc7b57be4283 100644
--- a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
@@ -1,9 +1,7 @@
 ; Check that MC/DC intrinsics are properly lowered
 ; RUN: opt < %s -passes=instrprof -S | FileCheck %s --check-prefixes=CHECK,BASIC
 ; RUN: opt < %s -passes=instrprof -S -instrprof-atomic-counter-update-all | FileCheck %s --check-prefixes=CHECK,ATOMIC
-; RUN: opt < %s -passes=instrprof -runtime-counter-relocation -S 2>&1 | FileCheck %s --check-prefix RELOC
-
-; RELOC: Runtime counter relocation is presently not supported for MC/DC bitmaps
+; RUN: opt < %s -passes=instrprof -S -runtime-counter-relocation | FileCheck %s --check-prefixes=CHECK,RELOC
 
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -14,10 +12,15 @@ target triple = "x86_64-unknown-linux-gnu"
 
 define dso_local void @test(i32 noundef %A) {
 entry:
+  ; RELOC: %profbm_bias = load i64, ptr @__llvm_profile_bitmap_bias, align [[#]], !invariant.load !0
+  ; RELOC: %profc_bias = load i64, ptr @__llvm_profile_counter_bias, align [[#]]
   %A.addr = alloca i32, align 4
   %mcdc.addr = alloca i32, align 4
   call void @llvm.instrprof.cover(ptr @__profn_test, i64 99278, i32 5, i32 0)
   ; BASIC: store i8 0, ptr @__profc_test, align 1
+  ; RELOC: %[[PROFC_INTADDR:.+]] = add i64 ptrtoint (ptr @__profc_test to i64), %profc_bias
+  ; RELOC: %[[PROFC_ADDR:.+]] = inttoptr i64 %[[PROFC_INTADDR]] to ptr
+  ; RELOC: store i8 0, ptr %[[PROFC_ADDR]], align 1
 
   call void @llvm.instrprof.mcdc.parameters(ptr @__profn_test, i64 99278, i32 1)
   store i32 0, ptr %mcdc.addr, align 4
@@ -25,6 +28,7 @@ entry:
   %tobool = icmp ne i32 %0, 0
 
   call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @__profn_test, i64 99278, i32 0, ptr %mcdc.addr)
+  ; RELOC:      [[PROFBM_ADDR:%.+]] = getelementptr i8, ptr @__profbm_test, i64 %profbm_bias
   ; CHECK:      %[[TEMP0:mcdc.*]] = load i32, ptr %mcdc.addr, align 4
   ; CHECK-NEXT: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 0
   ; CHECK-NEXT: %[[LAB4:[0-9]+]] = lshr i32 %[[TEMP]], 3
@@ -39,7 +43,9 @@ entry:
 ; CHECK: define private void @[[RMW_OR]](ptr %[[ARGPTR:.+]], i8 %[[ARGVAL:.+]])
 ; CHECK:      %[[BITS:.+]] = load i8, ptr %[[ARGPTR]], align 1
 ; BASIC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[ARGVAL]]
+; RELOC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[ARGVAL]]
 ; BASIC-NEXT: store i8 %[[LAB11]], ptr %[[ARGPTR]], align 1
+; RELOC-NEXT: store i8 %[[LAB11]], ptr %[[ARGPTR]], align 1
 ; ATOMIC-NEXT: %[[MASKED:.+]] = and i8 %[[BITS]], %[[ARGVAL]]
 ; ATOMIC-NEXT: %[[SHOULDWRITE:.+]] = icmp ne i8 %[[MASKED]], %[[ARGVAL]]
 ; ATOMIC-NEXT: br i1 %[[SHOULDWRITE]], label %[[WRITE:.+]], label %[[SKIP:.+]], !prof ![[MDPROF:[0-9]+]]

ornata · 2024-08-02T12:00:23Z

IIUC

Scope: Programs built with MC/DC coverage with continuous mode enabled.
Risk: This is a new feature for MC/DC coverage. Previously, MC/DC had no support for continuous mode. There should be no breakage introduced for existing features.
Testing: LLVM lit tests attached.

@chapuni What testing have you done other than LLVM lit tests? LLVM test suite? Clang itself?

chapuni

Let me self-approve at first :)

I've checked this functionality with running check-llvm on Linux. Note, %c is extremely slowere than non-continuous mode, even if %c%m8 on 48 cores.

In general, continuous mode tends to emit better score (than non-continuous mode), guessing it will cover also crashing tests.

vitalybuka · 2024-08-08T00:16:44Z

If this is new feature, why it needs a backport?

vitalybuka · 2024-08-08T00:24:14Z

I am out of context for this feature, leaving to @ornata

chapuni · 2024-08-08T04:21:38Z

Okay, I agree closing this. I'll propose yet another design till the next next release. :)

Nothing is progressing nor regressing, since MC/DC is unavailable with %c. Thank you.

chapuni added 3 commits August 2, 2024 07:59

Fixup (llvm#96126) mcdc.ll for aarch64. align is not 8.

aa2f055

(cherry picked from commit 891d898)

Fix for llvm#96126, add BitmapBiasAddr to definitions for unsupport…

36c6a91

…ed targets. (cherry picked from commit 20d723e)

llvmbot added this to the LLVM 19.X Release milestone Aug 2, 2024

llvmbot requested a review from chapuni August 2, 2024 08:00

llvmbot mentioned this pull request Aug 2, 2024

release/19.x: Please merge "bitmap bias" (#96126) for MC/DC to be available with continuous mode #101628

Closed

llvmbot added compiler-rt PGO Profile Guided Optimizations llvm:transforms labels Aug 2, 2024

chapuni changed the title ~~release/19.x: Fix for #96126, add BitmapBiasAddr to definitions for unsupported targets.~~ release/19.x: [MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode (#96126) Aug 2, 2024

AaronBallman requested review from MaskRay and vitalybuka August 2, 2024 12:39

chapuni approved these changes Aug 2, 2024

View reviewed changes

vitalybuka requested review from ornata and removed request for vitalybuka August 8, 2024 00:23

chapuni closed this Aug 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

release/19.x: [MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode (#96126) #101629

release/19.x: [MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode (#96126) #101629

Uh oh!

llvmbot commented Aug 2, 2024

Uh oh!

llvmbot commented Aug 2, 2024

Uh oh!

llvmbot commented Aug 2, 2024 •

edited

Loading

Uh oh!

ornata commented Aug 2, 2024

Uh oh!

chapuni left a comment

Uh oh!

vitalybuka commented Aug 8, 2024

Uh oh!

vitalybuka commented Aug 8, 2024

Uh oh!

chapuni commented Aug 8, 2024

Uh oh!

Uh oh!

release/19.x: [MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode (#96126) #101629

release/19.x: [MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode (#96126) #101629

Uh oh!

Conversation

llvmbot commented Aug 2, 2024

Uh oh!

llvmbot commented Aug 2, 2024

Uh oh!

llvmbot commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ornata commented Aug 2, 2024

Uh oh!

chapuni left a comment

Choose a reason for hiding this comment

Uh oh!

vitalybuka commented Aug 8, 2024

Uh oh!

vitalybuka commented Aug 8, 2024

Uh oh!

chapuni commented Aug 8, 2024

Uh oh!

Uh oh!

llvmbot commented Aug 2, 2024 •

edited

Loading