[MC/DC][Coverage] Introduce "Bitmap Bias" for continuous mode #96126
+67
−17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-pgo Author: NAKAMURA Takumi (chapuni) Changes
This doesn't depend on #95702 but I'd like to merge it. Full diff: https://github.com/llvm/llvm-project/pull/96126.diff 6 Files Affected:
diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index e9866d94b762c..6d2df2195c739 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
/* The variable that holds the name of the profile data
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index b88e0b4b0b2ab..805e152837a83 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -199,11 +199,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
@@ -214,10 +218,17 @@ COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
"/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;
@@ -228,6 +239,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(). */
@@ -238,11 +252,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;
+ uint64_t PaddingBytesAfterCounters =
+ __llvm_profile_get_num_padding_bytes(CountersSize);
+
/* Map the profile. */
char *Profile = (char *)mmap(NULL, FileSize, PROT_READ | PROT_WRITE,
MAP_SHARED, fileno(File), 0);
@@ -259,7 +280,15 @@ 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). */
+ if (NumBitmapBytes == 0)
+ return 0;
+
+ /* Update profbm_bias. */
+ uint64_t FileOffsetToBitmap =
+ CountersOffsetInBiasMode + CountersSize + PaddingBytesAfterCounters;
+ /* Update the profile fields based on the current mapping. */
+ INSTR_PROF_PROFILE_BITMAP_BIAS_VAR =
+ (intptr_t)Profile - (uintptr_t)BitmapBegin + FileOffsetToBitmap;
/* Return the memory allocated for counters to OS. */
lprofReleaseMemoryPagesToOS((uintptr_t)BitmapBegin, (uintptr_t)BitmapEnd);
@@ -618,8 +647,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 df79073da5b50..a6461250f7f13 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 e9866d94b762c..6d2df2195c739 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
/* The variable that holds the name of the profile data
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index b9f1fcdd9c233..9879fba86ae24 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -939,18 +939,21 @@ Value *InstrLowerer::getCounterAddress(InstrProfCntrInstBase *I) {
Value *InstrLowerer::getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I) {
auto *Bitmaps = getOrCreateRegionBitmaps(I);
- IRBuilder<> Builder(I);
+ if (!isRuntimeCounterRelocationEnabled())
+ return Bitmaps;
- 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));
- }
+ // Put BiasLI onto the entry block.
+ Type *Int64Ty = Type::getInt64Ty(M.getContext());
+ Function *Fn = I->getParent()->getParent();
+ IRBuilder<> EntryBuilder(&Fn->getEntryBlock().front());
+ auto *Bias = getOrCreateBiasVar(getInstrProfBitmapBiasVarName());
+ auto *BiasLI = EntryBuilder.CreateLoad(Int64Ty, Bias, "profbm_bias");
+ BiasLI->setMetadata(LLVMContext::MD_invariant_load,
+ MDNode::get(M.getContext(), std::nullopt));
- return Bitmaps;
+ // 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 4980b45f90c50..25e9ed90832b1 100644
--- a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
@@ -1,21 +1,24 @@
; Check that MC/DC intrinsics are properly lowered
-; RUN: opt < %s -passes=instrprof -S | FileCheck %s
-; 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 | FileCheck %s --check-prefixes=CHECK,BASIC
+; RUN: opt < %s -passes=instrprof -S -runtime-counter-relocation | FileCheck %s --check-prefixes=CHECK,RELOC
target triple = "x86_64-unknown-linux-gnu"
@__profn_test = private constant [4 x i8] c"test"
-; CHECK: @__profbm_test = private global [1 x i8] zeroinitializer, section "__llvm_prf_bits", comdat, align 1
+; BASIC: [[PROFBM_ADDR:@__profbm_test]] = private global [1 x i8] zeroinitializer, section "__llvm_prf_bits", comdat, align 1
define dso_local void @test(i32 noundef %A) {
entry:
+ ; RELOC: %profbm_bias = load i64, ptr @__llvm_profile_bitmap_bias, align 8, !invariant.load !0
+ ; RELOC: %profc_bias = load i64, ptr @__llvm_profile_counter_bias, align 8
%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)
- ; CHECK: store i8 0, ptr @__profc_test, align 1
+ ; 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
@@ -23,16 +26,19 @@ 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
- ; CHECK-NEXT: %[[LAB7:[0-9]+]] = getelementptr inbounds i8, ptr @__profbm_test, i32 %[[LAB4]]
+ ; CHECK-NEXT: %[[LAB7:[0-9]+]] = getelementptr inbounds i8, ptr [[PROFBM_ADDR]], i32 %[[LAB4]]
; CHECK-NEXT: %[[LAB8:[0-9]+]] = and i32 %[[TEMP]], 7
; CHECK-NEXT: %[[LAB9:[0-9]+]] = trunc i32 %[[LAB8]] to i8
; CHECK-NEXT: %[[LAB10:[0-9]+]] = shl i8 1, %[[LAB9]]
; CHECK-NEXT: %[[BITS:mcdc.*]] = load i8, ptr %[[LAB7]], align 1
- ; CHECK-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
- ; CHECK-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
+ ; BASIC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
+ ; RELOC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
+ ; BASIC-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
+ ; RELOC-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
ret void
}
|
|
@llvm/pr-subscribers-llvm-transforms Author: NAKAMURA Takumi (chapuni) Changes
This doesn't depend on #95702 but I'd like to merge it. Full diff: https://github.com/llvm/llvm-project/pull/96126.diff 6 Files Affected:
diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
index e9866d94b762c..6d2df2195c739 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
/* The variable that holds the name of the profile data
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index b88e0b4b0b2ab..805e152837a83 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -199,11 +199,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
@@ -214,10 +218,17 @@ COMPILER_RT_VISIBILITY extern intptr_t INSTR_PROF_PROFILE_COUNTER_BIAS_VAR;
"/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;
@@ -228,6 +239,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(). */
@@ -238,11 +252,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;
+ uint64_t PaddingBytesAfterCounters =
+ __llvm_profile_get_num_padding_bytes(CountersSize);
+
/* Map the profile. */
char *Profile = (char *)mmap(NULL, FileSize, PROT_READ | PROT_WRITE,
MAP_SHARED, fileno(File), 0);
@@ -259,7 +280,15 @@ 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). */
+ if (NumBitmapBytes == 0)
+ return 0;
+
+ /* Update profbm_bias. */
+ uint64_t FileOffsetToBitmap =
+ CountersOffsetInBiasMode + CountersSize + PaddingBytesAfterCounters;
+ /* Update the profile fields based on the current mapping. */
+ INSTR_PROF_PROFILE_BITMAP_BIAS_VAR =
+ (intptr_t)Profile - (uintptr_t)BitmapBegin + FileOffsetToBitmap;
/* Return the memory allocated for counters to OS. */
lprofReleaseMemoryPagesToOS((uintptr_t)BitmapBegin, (uintptr_t)BitmapEnd);
@@ -618,8 +647,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 df79073da5b50..a6461250f7f13 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 e9866d94b762c..6d2df2195c739 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
/* The variable that holds the name of the profile data
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index b9f1fcdd9c233..9879fba86ae24 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -939,18 +939,21 @@ Value *InstrLowerer::getCounterAddress(InstrProfCntrInstBase *I) {
Value *InstrLowerer::getBitmapAddress(InstrProfMCDCTVBitmapUpdate *I) {
auto *Bitmaps = getOrCreateRegionBitmaps(I);
- IRBuilder<> Builder(I);
+ if (!isRuntimeCounterRelocationEnabled())
+ return Bitmaps;
- 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));
- }
+ // Put BiasLI onto the entry block.
+ Type *Int64Ty = Type::getInt64Ty(M.getContext());
+ Function *Fn = I->getParent()->getParent();
+ IRBuilder<> EntryBuilder(&Fn->getEntryBlock().front());
+ auto *Bias = getOrCreateBiasVar(getInstrProfBitmapBiasVarName());
+ auto *BiasLI = EntryBuilder.CreateLoad(Int64Ty, Bias, "profbm_bias");
+ BiasLI->setMetadata(LLVMContext::MD_invariant_load,
+ MDNode::get(M.getContext(), std::nullopt));
- return Bitmaps;
+ // 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 4980b45f90c50..25e9ed90832b1 100644
--- a/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/mcdc.ll
@@ -1,21 +1,24 @@
; Check that MC/DC intrinsics are properly lowered
-; RUN: opt < %s -passes=instrprof -S | FileCheck %s
-; 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 | FileCheck %s --check-prefixes=CHECK,BASIC
+; RUN: opt < %s -passes=instrprof -S -runtime-counter-relocation | FileCheck %s --check-prefixes=CHECK,RELOC
target triple = "x86_64-unknown-linux-gnu"
@__profn_test = private constant [4 x i8] c"test"
-; CHECK: @__profbm_test = private global [1 x i8] zeroinitializer, section "__llvm_prf_bits", comdat, align 1
+; BASIC: [[PROFBM_ADDR:@__profbm_test]] = private global [1 x i8] zeroinitializer, section "__llvm_prf_bits", comdat, align 1
define dso_local void @test(i32 noundef %A) {
entry:
+ ; RELOC: %profbm_bias = load i64, ptr @__llvm_profile_bitmap_bias, align 8, !invariant.load !0
+ ; RELOC: %profc_bias = load i64, ptr @__llvm_profile_counter_bias, align 8
%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)
- ; CHECK: store i8 0, ptr @__profc_test, align 1
+ ; 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
@@ -23,16 +26,19 @@ 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
- ; CHECK-NEXT: %[[LAB7:[0-9]+]] = getelementptr inbounds i8, ptr @__profbm_test, i32 %[[LAB4]]
+ ; CHECK-NEXT: %[[LAB7:[0-9]+]] = getelementptr inbounds i8, ptr [[PROFBM_ADDR]], i32 %[[LAB4]]
; CHECK-NEXT: %[[LAB8:[0-9]+]] = and i32 %[[TEMP]], 7
; CHECK-NEXT: %[[LAB9:[0-9]+]] = trunc i32 %[[LAB8]] to i8
; CHECK-NEXT: %[[LAB10:[0-9]+]] = shl i8 1, %[[LAB9]]
; CHECK-NEXT: %[[BITS:mcdc.*]] = load i8, ptr %[[LAB7]], align 1
- ; CHECK-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
- ; CHECK-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
+ ; BASIC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
+ ; RELOC-NEXT: %[[LAB11:[0-9]+]] = or i8 %[[BITS]], %[[LAB10]]
+ ; BASIC-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
+ ; RELOC-NEXT: store i8 %[[LAB11]], ptr %[[LAB7]], align 1
ret void
}
|
Conflicts: llvm/test/Instrumentation/InstrProfiling/mcdc.ll
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Conflicts: llvm/test/Instrumentation/InstrProfiling/mcdc.ll
Conflicts: compiler-rt/lib/profile/InstrProfilingFile.c
|
This is something we discussed internally when reviewing the MC/DC changes. There are two different design directions we can take here: use a separate bias for the bimap (as done in this change), or use the same bias for both counters and bitmap. The downside of using separate biases is that it can have significant negative performance impact. We intentionally load the counter bias at the beginning of the function, with the motivation being that unless we're under register pressure, the register allocator will use the same register throughout the entire function, reducing the number of loads. This has been the case in practice from what we've observed. Using a second bias is likely going to increase register pressure, especially on register-starved architectures like x86, and require more frequent loads for the bias. A more efficient alternative would be to use a single bias for both counters and bitmap. That would require the counters and the bitmap section being at a fixed offset, ideally right next to each other. In ELF, we might be able to achieve this using |
|
@petrhosek Thanks for the comment. I don't think this would introduce serious performance regression, and I don't think coverage-instrumented codes would be sensitive of performance so much. I know it'd be better to use the single bias offset between counters and bitmaps. We might do in the future. This is suboptimal but practical, I suppose. It'd be the best if we could map and override the profraw into the existing profc and profbm, as Darwin is doing. I'll propose the bitmap for coverage in the near future. It will not require counters. |
|
Do we have an expected % performance impact? If perf is critical, could it be possible to only handle ELF for now, and support COFF + Mach-O later? My intuition says that most folks that care about MC/DC are going to primarily be working with ELF. |
|
Ping. This will affect only if mcdc coverage is enabled with |
ornata
approved these changes
Jul 29, 2024
| FileOffsetToCounters + CountersSize + PaddingBytesAfterCounters; | ||
| /* Update the profile fields based on the current mapping. */ | ||
| INSTR_PROF_PROFILE_BITMAP_BIAS_VAR = | ||
| (intptr_t)Profile - (uintptr_t)BitmapBegin + FileOffsetToBitmap; |
There was a problem hiding this comment.
Yes, it should be unaware of wrapping-around. It was copypasto from INSTR_PROF_PROFILE_COUNTER_BIAS_VAR.
| return Bitmaps; | ||
| // Put BiasLI onto the entry block. | ||
| Type *Int64Ty = Type::getInt64Ty(M.getContext()); | ||
| Function *Fn = I->getParent()->getParent(); |
There was a problem hiding this comment.
I can see more getParent()->getParent() than getFunction() in this file :)
|
I am certain that this change won't affect current valid configurations. I will merge this tomorrow, several hours after, unless seeing strong objections. |
chapuni
added a commit
that referenced
this pull request
Jul 31, 2024
chapuni
added a commit
that referenced
this pull request
Jul 31, 2024
clementval
pushed a commit
to clementval/llvm-project
that referenced
this pull request
Jul 31, 2024
…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 ```
clementval
pushed a commit
to clementval/llvm-project
that referenced
this pull request
Jul 31, 2024
clementval
pushed a commit
to clementval/llvm-project
that referenced
this pull request
Jul 31, 2024
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 2, 2024
…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)
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 2, 2024
(cherry picked from commit 891d898)
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 2, 2024
…ed targets. (cherry picked from commit 20d723e)
chapuni
added a commit
to chapuni/llvm-project
that referenced
this pull request
Aug 7, 2024
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
…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 ```
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
bherrera
pushed a commit
to misttech/mist-os
that referenced
this pull request
Aug 23, 2024
This prepares the library for when the upstream change llvm/llvm-project#96126 lands, adding the INSTR_PROF_PROFILE_BITMAP_BIAS_VAR macro and causing instrumented code to refer to that symbol. Change-Id: Ibfdd49226341dab34678e096c74a15bd505be661 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1075139 Reviewed-by: Petr Hosek <[email protected]> Commit-Queue: Auto-Submit <[email protected]> Fuchsia-Auto-Submit: Roland McGrath <[email protected]>
bherrera
pushed a commit
to misttech/integration
that referenced
this pull request
Aug 23, 2024
This prepares the library for when the upstream change llvm/llvm-project#96126 lands, adding the INSTR_PROF_PROFILE_BITMAP_BIAS_VAR macro and causing instrumented code to refer to that symbol. Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1075139 Original-Revision: 44f071f210923d8e6ba9144b8b8486c160544a60 GitOrigin-RevId: 74309e5440ef9d06b44f75216becded634524b32 Change-Id: Ic8211d421728753dbbe9c407f864aabfc5c6b24d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
counter_biasis incompatible to Bitmap. The distance between Counters and Bitmap is different between on-memory sections and profraw image.Reference to
__llvm_profile_bitmap_biasis generated only if-fcoverge-mcdc-runtime-counter-relocationare specified. The current implementation rejected their options.