From 779c05a62548b694750be985d86fa193b0983fa7 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Thu, 15 Jan 2026 09:08:06 +0900 Subject: [PATCH] [MC/DC] Create dedicated MCDCCondBitmapAddr for each Decision (#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. --- clang/lib/CodeGen/CodeGenFunction.h | 3 -- clang/lib/CodeGen/CodeGenPGO.cpp | 54 +++++++++++++++---- clang/lib/CodeGen/CodeGenPGO.h | 8 ++- clang/lib/CodeGen/MCDCState.h | 2 + clang/test/Profile/c-mcdc-logicalop-ternary.c | 18 ++++--- 5 files changed, 59 insertions(+), 26 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index caf68f28d466..1073de1d25ec 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1644,9 +1644,6 @@ private: std::unique_ptr 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; diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 9cd53208f8ab..d4d1abd50af4 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -1265,9 +1265,29 @@ void CodeGenPGO::emitMCDCParameters(CGBuilderTy &Builder) { CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_parameters), Args); } +/// Fill mcdc.addr order by ID. +std::vector
+CodeGenPGO::getMCDCCondBitmapAddrArray(CGBuilderTy &Builder) { + std::vector
Result; + + if (!canEmitMCDCCoverage(Builder) || !RegionMCDCState) + return Result; + + SmallVector> 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; @@ -1278,6 +1298,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) @@ -1300,14 +1324,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. @@ -1315,7 +1341,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) @@ -1345,6 +1370,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, @@ -1558,7 +1587,12 @@ void CodeGenFunction::markStmtMaybeUsed(const Stmt *S) { void CodeGenFunction::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"); } } bool CodeGenFunction::isMCDCDecisionExpr(const Expr *E) const { @@ -1569,13 +1603,13 @@ bool CodeGenFunction::isMCDCBranchExpr(const Expr *E) const { } void CodeGenFunction::maybeResetMCDCCondBitmap(const Expr *E) { if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) { - PGO->emitMCDCCondBitmapReset(Builder, E, MCDCCondBitmapAddr); + PGO->emitMCDCCondBitmapReset(Builder, E); PGO->setCurrentStmt(E); } } void CodeGenFunction::maybeUpdateMCDCTestVectorBitmap(const Expr *E) { if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) { - PGO->emitMCDCTestVectorBitmapUpdate(Builder, E, MCDCCondBitmapAddr, *this); + PGO->emitMCDCTestVectorBitmapUpdate(Builder, E, *this); PGO->setCurrentStmt(E); } } @@ -1583,7 +1617,7 @@ void CodeGenFunction::maybeUpdateMCDCTestVectorBitmap(const Expr *E) { void CodeGenFunction::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.h b/clang/lib/CodeGen/CodeGenPGO.h index 05cb9d497b91..5e2d72e8427f 100644 --- a/clang/lib/CodeGen/CodeGenPGO.h +++ b/clang/lib/CodeGen/CodeGenPGO.h @@ -129,14 +129,12 @@ public: bool UseFalsePath, bool UseBoth, 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
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 4b89ddd12d67..583157409b02 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/Profile/c-mcdc-logicalop-ternary.c b/clang/test/Profile/c-mcdc-logicalop-ternary.c index 18ce0b4e5dc1..18682ffdbfec 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]]