https://github.com/chapuni created
https://github.com/llvm/llvm-project/pull/125411
MCDCCondBitmapAddr is moved from `CodeGenFunction` into `MCDCState` and created
for each Decision.
In `maybeCreateMCDCCondBitmap`, Allocate bitmaps for all valid Decisions and
emit them order by ID, to prevent nondeterminism.
>From c74e5af3fb458230931d7cbba5d32e5999c38bf4 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4ci...@gmail.com>
Date: Sun, 2 Feb 2025 22:01:40 +0900
Subject: [PATCH] [MC/DC] Create dedicated MCDCCondBitmapAddr for each Decision
MCDCCondBitmapAddr is moved from `CodeGenFunction` into `MCDCState`
and created for each Decision.
In `maybeCreateMCDCCondBitmap`, Allocate bitmaps for all valid
Decisions and emit them order by ID, to prevent nondeterminism.
---
clang/lib/CodeGen/CodeGenFunction.h | 17 +--
clang/lib/CodeGen/CodeGenPGO.cpp | 41 +++++--
clang/lib/CodeGen/CodeGenPGO.h | 8 +-
clang/lib/CodeGen/MCDCState.h | 2 +
.../test/CoverageMapping/mcdc-single-cond.cpp | 101 ++++++++++++++++++
clang/test/Profile/c-mcdc-logicalop-ternary.c | 18 ++--
6 files changed, 160 insertions(+), 27 deletions(-)
create mode 100644 clang/test/CoverageMapping/mcdc-single-cond.cpp
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index e978cad4336238..20d81ba3a3bde1 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1628,9 +1628,6 @@ class CodeGenFunction : public CodeGenTypeCache {
CodeGenPGO PGO;
- /// Bitmap used by MC/DC to track condition outcomes of a boolean expression.
- Address MCDCCondBitmapAddr = Address::invalid();
-
/// Calculate branch weights appropriate for PGO data
llvm::MDNode *createProfileWeights(uint64_t TrueCount,
uint64_t FalseCount) const;
@@ -1669,8 +1666,12 @@ class CodeGenFunction : public CodeGenTypeCache {
void maybeCreateMCDCCondBitmap() {
if (isMCDCCoverageEnabled()) {
PGO.emitMCDCParameters(Builder);
- MCDCCondBitmapAddr =
- CreateIRTemp(getContext().UnsignedIntTy, "mcdc.addr");
+
+ // Set up MCDCCondBitmapAddr for each Decision.
+ // Note: This doesn't initialize Addrs in invalidated Decisions.
+ for (auto *MCDCCondBitmapAddr : PGO.getMCDCCondBitmapAddrArray(Builder))
+ *MCDCCondBitmapAddr =
+ CreateIRTemp(getContext().UnsignedIntTy, "mcdc.addr");
}
}
@@ -1682,7 +1683,7 @@ class CodeGenFunction : public CodeGenTypeCache {
/// Zero-init the MCDC temp value.
void maybeResetMCDCCondBitmap(const Expr *E) {
if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) {
- PGO.emitMCDCCondBitmapReset(Builder, E, MCDCCondBitmapAddr);
+ PGO.emitMCDCCondBitmapReset(Builder, E);
PGO.setCurrentStmt(E);
}
}
@@ -1691,7 +1692,7 @@ class CodeGenFunction : public CodeGenTypeCache {
/// If \p StepV is null, the default increment is 1.
void maybeUpdateMCDCTestVectorBitmap(const Expr *E) {
if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) {
- PGO.emitMCDCTestVectorBitmapUpdate(Builder, E, MCDCCondBitmapAddr,
*this);
+ PGO.emitMCDCTestVectorBitmapUpdate(Builder, E, *this);
PGO.setCurrentStmt(E);
}
}
@@ -1699,7 +1700,7 @@ class CodeGenFunction : public CodeGenTypeCache {
/// Update the MCDC temp value with the condition's evaluated result.
void maybeUpdateMCDCCondBitmap(const Expr *E, llvm::Value *Val) {
if (isMCDCCoverageEnabled()) {
- PGO.emitMCDCCondBitmapUpdate(Builder, E, MCDCCondBitmapAddr, Val, *this);
+ PGO.emitMCDCCondBitmapUpdate(Builder, E, Val, *this);
PGO.setCurrentStmt(E);
}
}
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 0331ff83e633f7..7d16f673ada419 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1245,9 +1245,29 @@ void CodeGenPGO::emitMCDCParameters(CGBuilderTy
&Builder) {
CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_parameters), Args);
}
+/// Fill mcdc.addr order by ID.
+std::vector<Address *>
+CodeGenPGO::getMCDCCondBitmapAddrArray(CGBuilderTy &Builder) {
+ std::vector<Address *> Result;
+
+ if (!canEmitMCDCCoverage(Builder) || !RegionMCDCState)
+ return Result;
+
+ SmallVector<std::pair<unsigned, Address *>> SortedPair;
+ for (auto &[_, V] : RegionMCDCState->DecisionByStmt)
+ if (V.isValid())
+ SortedPair.emplace_back(V.ID, &V.MCDCCondBitmapAddr);
+
+ llvm::sort(SortedPair);
+
+ for (auto &[_, MCDCCondBitmapAddr] : SortedPair)
+ Result.push_back(MCDCCondBitmapAddr);
+
+ return Result;
+}
+
void CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
const Expr *S,
- Address MCDCCondBitmapAddr,
CodeGenFunction &CGF) {
if (!canEmitMCDCCoverage(Builder) || !RegionMCDCState)
return;
@@ -1258,6 +1278,10 @@ void
CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
if (DecisionStateIter == RegionMCDCState->DecisionByStmt.end())
return;
+ auto &MCDCCondBitmapAddr = DecisionStateIter->second.MCDCCondBitmapAddr;
+ if (!MCDCCondBitmapAddr.isValid())
+ return;
+
// Don't create tvbitmap_update if the record is allocated but excluded.
// Or `bitmap |= (1 << 0)` would be wrongly executed to the next bitmap.
if (DecisionStateIter->second.Indices.size() == 0)
@@ -1280,14 +1304,16 @@ void
CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_tvbitmap_update), Args);
}
-void CodeGenPGO::emitMCDCCondBitmapReset(CGBuilderTy &Builder, const Expr *S,
- Address MCDCCondBitmapAddr) {
+void CodeGenPGO::emitMCDCCondBitmapReset(CGBuilderTy &Builder, const Expr *S) {
if (!canEmitMCDCCoverage(Builder) || !RegionMCDCState)
return;
- S = S->IgnoreParens();
+ auto I = RegionMCDCState->DecisionByStmt.find(S->IgnoreParens());
+ if (I == RegionMCDCState->DecisionByStmt.end())
+ return;
- if (!RegionMCDCState->DecisionByStmt.contains(S))
+ auto &MCDCCondBitmapAddr = I->second.MCDCCondBitmapAddr;
+ if (!MCDCCondBitmapAddr.isValid())
return;
// Emit intrinsic that resets a dedicated temporary value on the stack to 0.
@@ -1295,7 +1321,6 @@ void CodeGenPGO::emitMCDCCondBitmapReset(CGBuilderTy
&Builder, const Expr *S,
}
void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
- Address MCDCCondBitmapAddr,
llvm::Value *Val,
CodeGenFunction &CGF) {
if (!canEmitMCDCCoverage(Builder) || !RegionMCDCState)
@@ -1325,6 +1350,10 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy
&Builder, const Expr *S,
if (DecisionIter == RegionMCDCState->DecisionByStmt.end())
return;
+ auto &MCDCCondBitmapAddr = DecisionIter->second.MCDCCondBitmapAddr;
+ if (!MCDCCondBitmapAddr.isValid())
+ return;
+
const auto &TVIdxs = DecisionIter->second.Indices[Branch.ID];
auto *CurTV = Builder.CreateLoad(MCDCCondBitmapAddr,
diff --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h
index 1944b640951d5c..0d7dd13f611a93 100644
--- a/clang/lib/CodeGen/CodeGenPGO.h
+++ b/clang/lib/CodeGen/CodeGenPGO.h
@@ -114,14 +114,12 @@ class CodeGenPGO {
void emitCounterSetOrIncrement(CGBuilderTy &Builder, const Stmt *S,
llvm::Value *StepV);
void emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
- Address MCDCCondBitmapAddr,
CodeGenFunction &CGF);
void emitMCDCParameters(CGBuilderTy &Builder);
- void emitMCDCCondBitmapReset(CGBuilderTy &Builder, const Expr *S,
- Address MCDCCondBitmapAddr);
+ std::vector<Address *> getMCDCCondBitmapAddrArray(CGBuilderTy &Builder);
+ void emitMCDCCondBitmapReset(CGBuilderTy &Builder, const Expr *S);
void emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
- Address MCDCCondBitmapAddr, llvm::Value *Val,
- CodeGenFunction &CGF);
+ llvm::Value *Val, CodeGenFunction &CGF);
void markStmtAsUsed(bool Skipped, const Stmt *S) {
// Do nothing.
diff --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h
index 0b6f5f28235f43..2bb4eaf9fdfc83 100644
--- a/clang/lib/CodeGen/MCDCState.h
+++ b/clang/lib/CodeGen/MCDCState.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_LIB_CODEGEN_MCDCSTATE_H
#define LLVM_CLANG_LIB_CODEGEN_MCDCSTATE_H
+#include "Address.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ProfileData/Coverage/MCDCTypes.h"
@@ -38,6 +39,7 @@ struct State {
unsigned BitmapIdx;
IndicesTy Indices;
unsigned ID = InvalidID;
+ Address MCDCCondBitmapAddr = Address::invalid();
bool isValid() const { return ID != InvalidID; }
diff --git a/clang/test/CoverageMapping/mcdc-single-cond.cpp
b/clang/test/CoverageMapping/mcdc-single-cond.cpp
new file mode 100644
index 00000000000000..d6c694a63dd285
--- /dev/null
+++ b/clang/test/CoverageMapping/mcdc-single-cond.cpp
@@ -0,0 +1,101 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-disable-llvm-passes -emit-llvm -o %t2.ll %s | FileCheck %s
--check-prefixes=MM,MM2
+// RUN: FileCheck %s --check-prefixes=LL,LL2 < %t2.ll
+
+// LL: define{{.+}}func_cond{{.+}}(
+// MM: func_cond{{.*}}:
+int func_cond(bool a, bool b) {
+ // %mcdc.addr* are emitted by static order.
+ // LL: %[[MA4:mcdc.addr.*]] = alloca i32, align 4
+ // LL: %[[MA6:mcdc.addr.*]] = alloca i32, align 4
+ // LL: %[[MA7:mcdc.addr.*]] = alloca i32, align 4
+ // LL: %[[MA9:mcdc.addr.*]] = alloca i32, align 4
+ // LL: %[[MA10:mcdc.addr.*]] = alloca i32, align 4
+ // LL: call void @llvm.instrprof.mcdc.parameters(ptr @[[PROFN:.+]], i64
[[#H:]], i32 [[#BS:]])
+ int count = 0;
+ if (a)
+ // NB=2 Single cond
+ // MM2-NOT: Decision
+ ++count;
+ if (a ? true : false)
+ // NB=2,2 Wider decision comes first.
+ // MA2 has C:2
+ // MA3 has C:1
+ ++count;
+ if (a && b ? true : false)
+ // NB=2,3 Wider decision comes first.
+ // MM2: Decision,File 0, [[@LINE-2]]:7 -> [[#L:@LINE-2]]:13 = M:[[#I:3]],
C:2
+ // MM: Branch,File 0, [[#L]]:7 -> [[#L]]:8 = #6, (#0 - #6) [1,2,0]
+ // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #7, (#6 - #7) [2,0,0]
+ // LL: store i32 0, ptr %[[MA4]], align 4
+ // LL: = load i32, ptr %[[MA4]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA4]], align 4
+ // LL: = load i32, ptr %[[MA4]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA4]], align 4
+ // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]],
i64 [[#H]], i32 [[#B:0]], ptr %[[MA4]])
+ ++count;
+ while (a || true) {
+ // NB=3 BinOp only
+ // MM: Decision,File 0, [[@LINE-2]]:10 -> [[#L:@LINE-2]]:19 =
M:[[#I:I+3]], C:2
+ // MM: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = (#0 - #9), #9 [1,0,2]
+ // MM: Branch,File 0, [[#L]]:15 -> [[#L]]:19 = (#9 - #10), 0 [2,0,0]
+ // LL: store i32 0, ptr %[[MA6]], align 4
+ // LL: = load i32, ptr %[[MA6]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA6]], align 4
+ // LL: = load i32, ptr %[[MA6]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA6]], align 4
+ // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]],
i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA6]])
+ ++count;
+ break;
+ }
+ while (a || true ? false : true) {
+ // Wider decision comes first.
+ // MM2: Decision,File 0, [[@LINE-2]]:10 -> [[#L:@LINE-2]]:19 =
M:[[#I:I+3]], C:2
+ // MM: Branch,File 0, [[#L]]:10 -> [[#L]]:11 = ((#0 + #11) - #13), #13
[1,0,2]
+ // MM: Branch,File 0, [[#L]]:15 -> [[#L]]:19 = (#13 - #14), 0 [2,0,0]
+ // LL: store i32 0, ptr %[[MA7]], align 4
+ // LL: = load i32, ptr %[[MA7]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA7]], align 4
+ // LL: = load i32, ptr %[[MA7]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA7]], align 4
+ // LL: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]],
i64 [[#H]], i32 [[#B:B+3]], ptr %[[MA7]])
+ ++count;
+ }
+ do {
+ ++count;
+ } while (a && false);
+ // BinOp only
+ // MM: Decision,File 0, [[@LINE-2]]:12 -> [[#L:@LINE-2]]:22 =
M:[[#I:I+3]], C:2
+ // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #16, ((#0 + #15) - #16)
[1,2,0]
+ // MM: Branch,File 0, [[#L]]:17 -> [[#L]]:22 = 0, (#16 - #17) [2,0,0]
+ // LL: store i32 0, ptr %[[MA9]], align 4
+ // LL: = load i32, ptr %[[MA9]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA9]], align 4
+ // LL: = load i32, ptr %[[MA9]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA9]], align 4
+ // LL2: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64
[[#H]], i32 [[#B:B+3]], ptr %[[MA9]])
+ do {
+ ++count;
+ } while (a && false ? true : false);
+ // Wider decision comes first.
+ // MM2: Decision,File 0, [[@LINE-2]]:12 -> [[#L:@LINE-2]]:22 = M:15, C:2
+ // MM: Branch,File 0, [[#L]]:12 -> [[#L]]:13 = #20, ((#0 + #18) - #20)
[1,2,0]
+ // MM: Branch,File 0, [[#L]]:17 -> [[#L]]:22 = 0, (#20 - #21) [2,0,0]
+ // LL: store i32 0, ptr %[[MA10]], align 4
+ // LL: = load i32, ptr %[[MA10]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA10]], align 4
+ // LL: = load i32, ptr %[[MA10]], align 4
+ // LL: store i32 %{{.+}}, ptr %[[MA10]], align 4
+ // LL: call void @llvm.instrprof.mcdc.tvbitmap.update(ptr @[[PROFN]], i64
[[#H]], i32 [[#B:B+3]], ptr %[[MA10]])
+ // FIXME: Confirm (B+3==BS)
+ for (int i = 0; i < (a ? 2 : 1); ++i) {
+ // Simple nested decision (different column)
+ // MM2-NOT: Decision
+ // LL2-NOT: call void @llvm.instrprof.mcdc.tvbitmap.update
+ ++count;
+ }
+ for (int i = 0; i >= 4 ? false : true; ++i) {
+ // Wider decision comes first.
+ ++count;
+ }
+ return count;
+}
diff --git a/clang/test/Profile/c-mcdc-logicalop-ternary.c
b/clang/test/Profile/c-mcdc-logicalop-ternary.c
index 18ce0b4e5dc149..18682ffdbfec38 100644
--- a/clang/test/Profile/c-mcdc-logicalop-ternary.c
+++ b/clang/test/Profile/c-mcdc-logicalop-ternary.c
@@ -13,12 +13,14 @@ int test(int a, int b, int c, int d, int e, int f) {
// ALLOCATE MCDC TEMP AND ZERO IT.
// MCDC-LABEL: @test(
-// MCDC: %mcdc.addr = alloca i32, align 4
-// MCDC: store i32 0, ptr %mcdc.addr, align 4
+// MCDC: %[[ADDR0:mcdc.+]] = alloca i32, align 4
+// MCDC: %[[ADDR1:mcdc.+]] = alloca i32, align 4
+// MCDC: %[[ADDR2:mcdc.+]] = alloca i32, align 4
+// MCDC: store i32 0, ptr %[[ADDR0]], align 4
// TERNARY TRUE SHOULD UPDATE THE BITMAP WITH RESULT AT ELEMENT 0.
// MCDC-LABEL: cond.true:
-// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %[[ADDR0]], align 4
// MCDC: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 0
// MCDC: %[[LAB1:[0-9]+]] = lshr i32 %[[TEMP]], 3
// MCDC: %[[LAB4:[0-9]+]] = getelementptr inbounds i8, ptr @__profbm_test,
i32 %[[LAB1]]
@@ -30,12 +32,12 @@ int test(int a, int b, int c, int d, int e, int f) {
// MCDC: store i8 %[[LAB9]], ptr %[[LAB4]], align 1
// CHECK FOR ZERO OF MCDC TEMP
-// MCDC: store i32 0, ptr %mcdc.addr, align 4
+// MCDC: store i32 0, ptr %[[ADDR1]], align 4
// TERNARY TRUE YIELDS TERNARY LHS LOGICAL-AND.
// TERNARY LHS LOGICAL-AND SHOULD UPDATE THE BITMAP WITH RESULT AT ELEMENT 1.
// MCDC-LABEL: land.end:
-// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %[[ADDR1]], align 4
// MCDC: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 3
// MCDC: %[[LAB1:[0-9]+]] = lshr i32 %[[TEMP]], 3
// MCDC: %[[LAB4:[0-9]+]] = getelementptr inbounds i8, ptr @__profbm_test,
i32 %[[LAB1]]
@@ -48,7 +50,7 @@ int test(int a, int b, int c, int d, int e, int f) {
// TERNARY FALSE SHOULD UPDATE THE BITMAP WITH RESULT AT ELEMENT 0.
// MCDC-LABEL: cond.false:
-// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG: %[[TEMP0:mcdc.+]] = load i32, ptr %[[ADDR0]], align 4
// MCDC: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 0
// MCDC: %[[LAB1:[0-9]+]] = lshr i32 %[[TEMP]], 3
// MCDC: %[[LAB4:[0-9]+]] = getelementptr inbounds i8, ptr @__profbm_test,
i32 %[[LAB1]]
@@ -60,12 +62,12 @@ int test(int a, int b, int c, int d, int e, int f) {
// MCDC: store i8 %[[LAB9]], ptr %[[LAB4]], align 1
// CHECK FOR ZERO OF MCDC TEMP
-// MCDC: store i32 0, ptr %mcdc.addr, align 4
+// MCDC: store i32 0, ptr %[[ADDR2]], align 4
// TERNARY FALSE YIELDS TERNARY RHS LOGICAL-OR.
// TERNARY RHS LOGICAL-OR SHOULD UPDATE THE BITMAP WITH RESULT AT ELEMENT 2.
// MCDC-LABEL: lor.end:
-// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %mcdc.addr, align 4
+// MCDC-DAG: %[[TEMP0:mcdc.temp[0-9]*]] = load i32, ptr %[[ADDR2]], align 4
// MCDC: %[[TEMP:[0-9]+]] = add i32 %[[TEMP0]], 6
// MCDC: %[[LAB1:[0-9]+]] = lshr i32 %[[TEMP]], 3
// MCDC: %[[LAB4:[0-9]+]] = getelementptr inbounds i8, ptr @__profbm_test,
i32 %[[LAB1]]
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
