[Coverage] ProfileData: Handle MC/DC Bitmap as BitVector. NFC. (#80608)

* `getFunctionBitmap()` stores not `std::vector<uint8_t>` but
`BitVector`.
* `CounterMappingContext` holds `Bitmap` (instead of the ref of bytes)
* `Bitmap` and `BitmapIdx` are used instead of `evaluateBitmap()`.

FIXME: `InstrProfRecord` itself should handle `Bitmap` as `BitVector`.

Author: chapuni

llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h

@@ -562,15 +562,15 @@ struct MCDCRecord {
 class CounterMappingContext {
   ArrayRef<CounterExpression> Expressions;
   ArrayRef<uint64_t> CounterValues;
-  ArrayRef<uint8_t> BitmapBytes;
+  BitVector Bitmap;
 
 public:
   CounterMappingContext(ArrayRef<CounterExpression> Expressions,
                         ArrayRef<uint64_t> CounterValues = std::nullopt)
       : Expressions(Expressions), CounterValues(CounterValues) {}
 
   void setCounts(ArrayRef<uint64_t> Counts) { CounterValues = Counts; }
-  void setBitmapBytes(ArrayRef<uint8_t> Bytes) { BitmapBytes = Bytes; }
+  void setBitmap(BitVector &&Bitmap_) { Bitmap = std::move(Bitmap_); }
 
   void dump(const Counter &C, raw_ostream &OS) const;
   void dump(const Counter &C) const { dump(C, dbgs()); }
@@ -579,16 +579,10 @@ class CounterMappingContext {
   /// counter was executed.
   Expected<int64_t> evaluate(const Counter &C) const;
 
-  /// Return the number of times that a region of code associated with this
-  /// counter was executed.
-  Expected<BitVector>
-  evaluateBitmap(const CounterMappingRegion *MCDCDecision) const;
-
   /// Return an MCDC record that indicates executed test vectors and condition
   /// pairs.
   Expected<MCDCRecord>
   evaluateMCDCRegion(const CounterMappingRegion &Region,
-                     const BitVector &Bitmap,
                      ArrayRef<const CounterMappingRegion *> Branches);
 
   unsigned getMaxCounterID(const Counter &C) const;

llvm/include/llvm/ProfileData/InstrProfReader.h

@@ -736,9 +736,9 @@ class IndexedInstrProfReader : public InstrProfReader {
   Error getFunctionCounts(StringRef FuncName, uint64_t FuncHash,
                           std::vector<uint64_t> &Counts);
 
-  /// Fill Bitmap Bytes with the profile data for the given function name.
-  Error getFunctionBitmapBytes(StringRef FuncName, uint64_t FuncHash,
-                               std::vector<uint8_t> &BitmapBytes);
+  /// Fill Bitmap with the profile data for the given function name.
+  Error getFunctionBitmap(StringRef FuncName, uint64_t FuncHash,
+                          BitVector &Bitmap);
 
   /// Return the maximum of all known function counts.
   /// \c UseCS indicates whether to use the context-sensitive count.

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

@@ -223,33 +223,12 @@ Expected<int64_t> CounterMappingContext::evaluate(const Counter &C) const {
   return LastPoppedValue;
 }
 
-Expected<BitVector> CounterMappingContext::evaluateBitmap(
-    const CounterMappingRegion *MCDCDecision) const {
-  unsigned ID = MCDCDecision->MCDCParams.BitmapIdx;
-  unsigned NC = MCDCDecision->MCDCParams.NumConditions;
-  unsigned SizeInBits = llvm::alignTo(uint64_t(1) << NC, CHAR_BIT);
-  unsigned SizeInBytes = SizeInBits / CHAR_BIT;
-
-  assert(ID + SizeInBytes <= BitmapBytes.size() && "BitmapBytes overrun");
-  ArrayRef<uint8_t> Bytes(&BitmapBytes[ID], SizeInBytes);
-
-  // Mask each bitmap byte into the BitVector. Go in reverse so that the
-  // bitvector can just be shifted over by one byte on each iteration.
-  BitVector Result(SizeInBits, false);
-  for (auto Byte = std::rbegin(Bytes); Byte != std::rend(Bytes); ++Byte) {
-    uint32_t Data = *Byte;
-    Result <<= CHAR_BIT;
-    Result.setBitsInMask(&Data, 1);
-  }
-  return Result;
-}
-
 class MCDCRecordProcessor {
   /// A bitmap representing the executed test vectors for a boolean expression.
   /// Each index of the bitmap corresponds to a possible test vector. An index
   /// with a bit value of '1' indicates that the corresponding Test Vector
   /// identified by that index was executed.
-  const BitVector &ExecutedTestVectorBitmap;
+  const BitVector &Bitmap;
 
   /// Decision Region to which the ExecutedTestVectorBitmap applies.
   const CounterMappingRegion &Region;
@@ -261,6 +240,8 @@ class MCDCRecordProcessor {
   /// Total number of conditions in the boolean expression.
   unsigned NumConditions;
 
+  unsigned BitmapIdx;
+
   /// Mapping of a condition ID to its corresponding branch region.
   llvm::DenseMap<unsigned, const CounterMappingRegion *> Map;
 
@@ -281,8 +262,9 @@ class MCDCRecordProcessor {
   MCDCRecordProcessor(const BitVector &Bitmap,
                       const CounterMappingRegion &Region,
                       ArrayRef<const CounterMappingRegion *> Branches)
-      : ExecutedTestVectorBitmap(Bitmap), Region(Region), Branches(Branches),
+      : Bitmap(Bitmap), Region(Region), Branches(Branches),
         NumConditions(Region.MCDCParams.NumConditions),
+        BitmapIdx(Region.MCDCParams.BitmapIdx * CHAR_BIT),
         Folded(NumConditions, false), IndependencePairs(NumConditions),
         TestVectors((size_t)1 << NumConditions) {}
 
@@ -323,9 +305,10 @@ class MCDCRecordProcessor {
 
   /// Walk the bits in the bitmap.  A bit set to '1' indicates that the test
   /// vector at the corresponding index was executed during a test run.
-  void findExecutedTestVectors(const BitVector &ExecutedTestVectorBitmap) {
-    for (unsigned Idx = 0; Idx < ExecutedTestVectorBitmap.size(); ++Idx) {
-      if (ExecutedTestVectorBitmap[Idx] == 0)
+  void findExecutedTestVectors() {
+    for (unsigned Idx = 0; Idx < (1u << NumConditions); ++Idx) {
+      assert(BitmapIdx + Idx < Bitmap.size() && "Bitmap overrun");
+      if (Bitmap[BitmapIdx + Idx] == 0)
         continue;
       assert(!TestVectors[Idx].empty() && "Test Vector doesn't exist.");
       ExecVectors.push_back(TestVectors[Idx]);
@@ -402,7 +385,7 @@ class MCDCRecordProcessor {
     buildTestVector(TV, 1, 0);
 
     // Using Profile Bitmap from runtime, mark the executed test vectors.
-    findExecutedTestVectors(ExecutedTestVectorBitmap);
+    findExecutedTestVectors();
 
     // Compare executed test vectors against each other to find an independence
     // pairs for each condition.  This processing takes the most time.
@@ -417,10 +400,9 @@ class MCDCRecordProcessor {
 
 Expected<MCDCRecord> CounterMappingContext::evaluateMCDCRegion(
     const CounterMappingRegion &Region,
-    const BitVector &ExecutedTestVectorBitmap,
     ArrayRef<const CounterMappingRegion *> Branches) {
 
-  MCDCRecordProcessor MCDCProcessor(ExecutedTestVectorBitmap, Region, Branches);
+  MCDCRecordProcessor MCDCProcessor(Bitmap, Region, Branches);
   return MCDCProcessor.processMCDCRecord();
 }
 
@@ -506,6 +488,7 @@ static unsigned getMaxCounterID(const CounterMappingContext &Ctx,
   return MaxCounterID;
 }
 
+/// Returns the bit count
 static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
                                  const CoverageMappingRecord &Record) {
   unsigned MaxBitmapID = 0;
@@ -521,7 +504,7 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
     }
   }
   unsigned SizeInBits = llvm::alignTo(uint64_t(1) << NumConditions, CHAR_BIT);
-  return MaxBitmapID + (SizeInBits / CHAR_BIT);
+  return MaxBitmapID * CHAR_BIT + SizeInBits;
 }
 
 namespace {
@@ -708,9 +691,9 @@ Error CoverageMapping::loadFunctionRecord(
   }
   Ctx.setCounts(Counts);
 
-  std::vector<uint8_t> BitmapBytes;
-  if (Error E = ProfileReader.getFunctionBitmapBytes(
-          Record.FunctionName, Record.FunctionHash, BitmapBytes)) {
+  BitVector Bitmap;
+  if (Error E = ProfileReader.getFunctionBitmap(Record.FunctionName,
+                                                Record.FunctionHash, Bitmap)) {
     instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
     if (IPE == instrprof_error::hash_mismatch) {
       FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
@@ -719,9 +702,9 @@ Error CoverageMapping::loadFunctionRecord(
     }
     if (IPE != instrprof_error::unknown_function)
       return make_error<InstrProfError>(IPE);
-    BitmapBytes.assign(getMaxBitmapSize(Ctx, Record) + 1, 0);
+    Bitmap = BitVector(getMaxBitmapSize(Ctx, Record));
   }
-  Ctx.setBitmapBytes(BitmapBytes);
+  Ctx.setBitmap(std::move(Bitmap));
 
   assert(!Record.MappingRegions.empty() && "Function has no regions");
 
@@ -772,23 +755,11 @@ Error CoverageMapping::loadFunctionRecord(
     auto MCDCDecision = Result->first;
     auto &MCDCBranches = Result->second;
 
-    // Evaluating the test vector bitmap for the decision region entails
-    // calculating precisely what bits are pertinent to this region alone.
-    // This is calculated based on the recorded offset into the global
-    // profile bitmap; the length is calculated based on the recorded
-    // number of conditions.
-    Expected<BitVector> ExecutedTestVectorBitmap =
-        Ctx.evaluateBitmap(MCDCDecision);
-    if (auto E = ExecutedTestVectorBitmap.takeError()) {
-      consumeError(std::move(E));
-      return Error::success();
-    }
-
     // Since the bitmap identifies the executed test vectors for an MC/DC
     // DecisionRegion, all of the information is now available to process.
     // This is where the bulk of the MC/DC progressing takes place.
-    Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
-        *MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
+    Expected<MCDCRecord> Record =
+        Ctx.evaluateMCDCRegion(*MCDCDecision, MCDCBranches);
     if (auto E = Record.takeError()) {
       consumeError(std::move(E));
       return Error::success();

llvm/lib/ProfileData/InstrProfReader.cpp

@@ -1435,13 +1435,27 @@ Error IndexedInstrProfReader::getFunctionCounts(StringRef FuncName,
   return success();
 }
 
-Error IndexedInstrProfReader::getFunctionBitmapBytes(
-    StringRef FuncName, uint64_t FuncHash, std::vector<uint8_t> &BitmapBytes) {
+Error IndexedInstrProfReader::getFunctionBitmap(StringRef FuncName,
+                                                uint64_t FuncHash,
+                                                BitVector &Bitmap) {
   Expected<InstrProfRecord> Record = getInstrProfRecord(FuncName, FuncHash);
   if (Error E = Record.takeError())
     return error(std::move(E));
 
-  BitmapBytes = Record.get().BitmapBytes;
+  const auto &BitmapBytes = Record.get().BitmapBytes;
+  size_t I = 0, E = BitmapBytes.size();
+  Bitmap.resize(E * CHAR_BIT);
+  BitVector::apply(
+      [&](auto x) {
+        decltype(x) W = 0;
+        size_t N = std::min(E - I, sizeof(W));
+        std::memcpy((void *)&W, &BitmapBytes[I], N);
+        I += N;
+        return W;
+      },
+      Bitmap, Bitmap);
+  assert(I == E);
+
   return success();
 }