[clang] 1f6a347 - Refactor: Let MCDC::State have DecisionByStmt and BranchByStmt

NAKAMURA Takumi via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 25 01:34:24 PST 2024


Author: NAKAMURA Takumi
Date: 2024-02-25T18:33:53+09:00
New Revision: 1f6a347c8abf8868fb4630c404480226c2efc2c2

URL: https://github.com/llvm/llvm-project/commit/1f6a347c8abf8868fb4630c404480226c2efc2c2
DIFF: https://github.com/llvm/llvm-project/commit/1f6a347c8abf8868fb4630c404480226c2efc2c2.diff

LOG: Refactor: Let MCDC::State have DecisionByStmt and BranchByStmt

- Prune `RegionMCDCBitmapMap` and `RegionCondIDMap`. They are handled
  by `MCDCState`.
- Rename `s/BitmapMap/DecisionByStmt/`. It can handle Decision stuff.
- Rename `s/CondIDMap/BranchByStmt/`. It can be handle Branch stuff.
- `MCDCRecordProcessor`: Use `DecisionParams.BitmapIdx` directly.

Added: 
    

Modified: 
    clang/lib/CodeGen/CodeGenPGO.cpp
    clang/lib/CodeGen/CodeGenPGO.h
    clang/lib/CodeGen/CoverageMappingGen.cpp
    clang/lib/CodeGen/MCDCState.h
    llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 1ef7be3c72593d..8aebd3557690af 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -165,6 +165,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
   llvm::DenseMap<const Stmt *, unsigned> &CounterMap;
   /// The next bitmap byte index to assign.
   unsigned NextMCDCBitmapIdx;
+  /// The state of MC/DC Coverage in this function.
   MCDC::State &MCDCState;
   /// Maximum number of supported MC/DC conditions in a boolean expression.
   unsigned MCDCMaxCond;
@@ -311,7 +312,7 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
 
           // Otherwise, allocate the number of bytes required for the bitmap
           // based on the number of conditions. Must be at least 1-byte long.
-          MCDCState.BitmapMap[BinOp] = NextMCDCBitmapIdx;
+          MCDCState.DecisionByStmt[BinOp].BitmapIdx = NextMCDCBitmapIdx;
           unsigned SizeInBits = std::max<unsigned>(1L << NumCond, CHAR_BIT);
           NextMCDCBitmapIdx += SizeInBits / CHAR_BIT;
         }
@@ -1034,7 +1035,7 @@ void CodeGenPGO::emitCounterRegionMapping(const Decl *D) {
 
   std::string CoverageMapping;
   llvm::raw_string_ostream OS(CoverageMapping);
-  RegionCondIDMap.reset(new llvm::DenseMap<const Stmt *, int16_t>);
+  RegionMCDCState->BranchByStmt.clear();
   CoverageMappingGen MappingGen(
       *CGM.getCoverageMapping(), CGM.getContext().getSourceManager(),
       CGM.getLangOpts(), RegionCounterMap.get(), RegionMCDCState.get());
@@ -1142,12 +1143,12 @@ void CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
 
   S = S->IgnoreParens();
 
-  auto ExprMCDCBitmapMapIterator = RegionMCDCState->BitmapMap.find(S);
-  if (ExprMCDCBitmapMapIterator == RegionMCDCState->BitmapMap.end())
+  auto DecisionStateIter = RegionMCDCState->DecisionByStmt.find(S);
+  if (DecisionStateIter == RegionMCDCState->DecisionByStmt.end())
     return;
 
-  // Extract the ID of the global bitmap associated with this expression.
-  unsigned MCDCTestVectorBitmapID = ExprMCDCBitmapMapIterator->second;
+  // Extract the offset of the global bitmap associated with this expression.
+  unsigned MCDCTestVectorBitmapOffset = DecisionStateIter->second.BitmapIdx;
   auto *I8PtrTy = llvm::PointerType::getUnqual(CGM.getLLVMContext());
 
   // Emit intrinsic responsible for updating the global bitmap corresponding to
@@ -1158,7 +1159,7 @@ void CodeGenPGO::emitMCDCTestVectorBitmapUpdate(CGBuilderTy &Builder,
   llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
                           Builder.getInt64(FunctionHash),
                           Builder.getInt32(RegionMCDCState->BitmapBytes),
-                          Builder.getInt32(MCDCTestVectorBitmapID),
+                          Builder.getInt32(MCDCTestVectorBitmapOffset),
                           MCDCCondBitmapAddr.getPointer()};
   Builder.CreateCall(
       CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_tvbitmap_update), Args);
@@ -1171,7 +1172,7 @@ void CodeGenPGO::emitMCDCCondBitmapReset(CGBuilderTy &Builder, const Expr *S,
 
   S = S->IgnoreParens();
 
-  if (!RegionMCDCState->BitmapMap.contains(S))
+  if (!RegionMCDCState->DecisionByStmt.contains(S))
     return;
 
   // Emit intrinsic that resets a dedicated temporary value on the stack to 0.
@@ -1193,13 +1194,13 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
   // also make debugging a bit easier.
   S = CodeGenFunction::stripCond(S);
 
-  auto ExprMCDCConditionIDMapIterator = RegionMCDCState->CondIDMap.find(S);
-  if (ExprMCDCConditionIDMapIterator == RegionMCDCState->CondIDMap.end())
+  auto BranchStateIter = RegionMCDCState->BranchByStmt.find(S);
+  if (BranchStateIter == RegionMCDCState->BranchByStmt.end())
     return;
 
   // Extract the ID of the condition we are setting in the bitmap.
-  auto CondID = ExprMCDCConditionIDMapIterator->second;
-  assert(CondID >= 0 && "Condition has no ID!");
+  const auto &Branch = BranchStateIter->second;
+  assert(Branch.ID >= 0 && "Condition has no ID!");
 
   auto *I8PtrTy = llvm::PointerType::getUnqual(CGM.getLLVMContext());
 
@@ -1208,7 +1209,7 @@ void CodeGenPGO::emitMCDCCondBitmapUpdate(CGBuilderTy &Builder, const Expr *S,
   // the resulting value is used to update the boolean expression's bitmap.
   llvm::Value *Args[5] = {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
                           Builder.getInt64(FunctionHash),
-                          Builder.getInt32(CondID),
+                          Builder.getInt32(Branch.ID),
                           MCDCCondBitmapAddr.getPointer(), Val};
   Builder.CreateCall(
       CGM.getIntrinsic(llvm::Intrinsic::instrprof_mcdc_condbitmap_update),

diff  --git a/clang/lib/CodeGen/CodeGenPGO.h b/clang/lib/CodeGen/CodeGenPGO.h
index 369bf05b59a0d2..d3c2b277238fc7 100644
--- a/clang/lib/CodeGen/CodeGenPGO.h
+++ b/clang/lib/CodeGen/CodeGenPGO.h
@@ -36,8 +36,6 @@ class CodeGenPGO {
   unsigned NumRegionCounters;
   uint64_t FunctionHash;
   std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionCounterMap;
-  std::unique_ptr<llvm::DenseMap<const Stmt *, unsigned>> RegionMCDCBitmapMap;
-  std::unique_ptr<llvm::DenseMap<const Stmt *, int16_t>> RegionCondIDMap;
   std::unique_ptr<llvm::DenseMap<const Stmt *, uint64_t>> StmtCountMap;
   std::unique_ptr<llvm::InstrProfRecord> ProfRecord;
   std::unique_ptr<MCDC::State> RegionMCDCState;

diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index d8fa69d825b8d6..d98ab79eac3aed 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -684,7 +684,6 @@ struct MCDCCoverageBuilder {
 
   llvm::SmallVector<mcdc::ConditionIDs> DecisionStack;
   MCDC::State &MCDCState;
-  llvm::DenseMap<const Stmt *, mcdc::ConditionID> &CondIDs;
   mcdc::ConditionID NextID = 0;
   bool NotMapped = false;
 
@@ -699,8 +698,8 @@ struct MCDCCoverageBuilder {
 
 public:
   MCDCCoverageBuilder(CodeGenModule &CGM, MCDC::State &MCDCState)
-      : CGM(CGM), DecisionStack(1, DecisionStackSentinel), MCDCState(MCDCState),
-        CondIDs(MCDCState.CondIDMap) {}
+      : CGM(CGM), DecisionStack(1, DecisionStackSentinel),
+        MCDCState(MCDCState) {}
 
   /// Return whether the build of the control flow map is at the top-level
   /// (root) of a logical operator nest in a boolean expression prior to the
@@ -714,16 +713,16 @@ struct MCDCCoverageBuilder {
 
   /// Set the given condition's ID.
   void setCondID(const Expr *Cond, mcdc::ConditionID ID) {
-    CondIDs[CodeGenFunction::stripCond(Cond)] = ID;
+    MCDCState.BranchByStmt[CodeGenFunction::stripCond(Cond)].ID = ID;
   }
 
   /// Return the ID of a given condition.
   mcdc::ConditionID getCondID(const Expr *Cond) const {
-    auto I = CondIDs.find(CodeGenFunction::stripCond(Cond));
-    if (I == CondIDs.end())
+    auto I = MCDCState.BranchByStmt.find(CodeGenFunction::stripCond(Cond));
+    if (I == MCDCState.BranchByStmt.end())
       return -1;
     else
-      return I->second;
+      return I->second.ID;
   }
 
   /// Return the LHS Decision ([0,0] if not set).
@@ -738,7 +737,7 @@ struct MCDCCoverageBuilder {
 
     // If binary expression is disqualified, don't do mapping.
     if (!isBuilding() &&
-        !MCDCState.BitmapMap.contains(CodeGenFunction::stripCond(E)))
+        !MCDCState.DecisionByStmt.contains(CodeGenFunction::stripCond(E)))
       NotMapped = true;
 
     // Don't go any further if we don't need to map condition IDs.
@@ -750,7 +749,7 @@ struct MCDCCoverageBuilder {
     // If the operator itself has an assigned ID, this means it represents a
     // larger subtree.  In this case, assign that ID to its LHS node.  Its RHS
     // will receive a new ID below. Otherwise, assign ID+1 to LHS.
-    if (CondIDs.contains(CodeGenFunction::stripCond(E)))
+    if (MCDCState.BranchByStmt.contains(CodeGenFunction::stripCond(E)))
       setCondID(E->getLHS(), getCondID(E));
     else
       setCondID(E->getLHS(), NextID++);
@@ -853,7 +852,9 @@ struct CounterCoverageMappingBuilder
     return Counter::getCounter(CounterMap[S]);
   }
 
-  unsigned getRegionBitmap(const Stmt *S) { return MCDCState.BitmapMap[S]; }
+  auto getBitmapIdx(const Stmt *S) {
+    return MCDCState.DecisionByStmt[S].BitmapIdx;
+  }
 
   /// Push a region onto the stack.
   ///
@@ -1984,7 +1985,7 @@ struct CounterCoverageMappingBuilder
     // Create MCDC Decision Region if at top-level (root).
     unsigned NumConds = 0;
     if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E)))
-      createDecisionRegion(E, getRegionBitmap(E), NumConds);
+      createDecisionRegion(E, getBitmapIdx(E), NumConds);
 
     // Extract the RHS's Execution Counter.
     Counter RHSExecCnt = getRegionCounter(E);
@@ -2037,7 +2038,7 @@ struct CounterCoverageMappingBuilder
     // Create MCDC Decision Region if at top-level (root).
     unsigned NumConds = 0;
     if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E)))
-      createDecisionRegion(E, getRegionBitmap(E), NumConds);
+      createDecisionRegion(E, getBitmapIdx(E), NumConds);
 
     // Extract the RHS's Execution Counter.
     Counter RHSExecCnt = getRegionCounter(E);

diff  --git a/clang/lib/CodeGen/MCDCState.h b/clang/lib/CodeGen/MCDCState.h
index e6e39237a1b41a..3b86cd9cedeea7 100644
--- a/clang/lib/CodeGen/MCDCState.h
+++ b/clang/lib/CodeGen/MCDCState.h
@@ -27,8 +27,18 @@ using namespace llvm::coverage::mcdc;
 /// Per-Function MC/DC state
 struct State {
   unsigned BitmapBytes = 0;
-  llvm::DenseMap<const Stmt *, unsigned> BitmapMap;
-  llvm::DenseMap<const Stmt *, ConditionID> CondIDMap;
+
+  struct Decision {
+    unsigned BitmapIdx;
+  };
+
+  llvm::DenseMap<const Stmt *, Decision> DecisionByStmt;
+
+  struct Branch {
+    ConditionID ID;
+  };
+
+  llvm::DenseMap<const Stmt *, Branch> BranchByStmt;
 };
 
 } // namespace clang::CodeGen::MCDC

diff  --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index ddce7580729170..8f9d1eadc2dafc 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -243,8 +243,6 @@ class MCDCRecordProcessor {
   /// Total number of conditions in the boolean expression.
   unsigned NumConditions;
 
-  unsigned BitmapIdx;
-
   /// Mapping of a condition ID to its corresponding branch params.
   llvm::DenseMap<unsigned, mcdc::ConditionIDs> CondsMap;
 
@@ -265,7 +263,6 @@ class MCDCRecordProcessor {
       : Bitmap(Bitmap), Region(Region),
         DecisionParams(Region.getDecisionParams()), Branches(Branches),
         NumConditions(DecisionParams.NumConditions),
-        BitmapIdx(DecisionParams.BitmapIdx * CHAR_BIT),
         Folded(NumConditions, false), IndependencePairs(NumConditions) {}
 
 private:
@@ -287,7 +284,7 @@ class MCDCRecordProcessor {
         continue;
       }
 
-      if (!Bitmap[BitmapIdx + Index])
+      if (!Bitmap[DecisionParams.BitmapIdx * CHAR_BIT + Index])
         continue;
 
       // Copy the completed test vector to the vector of testvectors.


        


More information about the cfe-commits mailing list