[utils] Move BitVector's iterator to STL-like interface.

Replace the Advance/Done methods on BitVector::Iterator with
STL-compatible operator overloads, and add begin/end methods to
BitVector itself, so that BitVectors can be iterated with ranged for
loops.

As a drive-by cleanup, make GrowableBitVector hold the BitVector by
value (to avoid needing to allocate one for empty iteration), and remove
its unused (and inefficient) Union method.

Change-Id: Idcd34e26bfb087e3ec8297b4a769a51bfab4b6e8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3455803Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79071}

src/compiler/backend/mid-tier-register-allocator.cc

@@ -3240,10 +3240,8 @@ void MidTierRegisterAllocator::UpdateSpillRangesForLoops() {
           data_->GetBlock(last_loop_block)->last_instruction_index();
       // Extend spill range for all spilled values that are live on entry to the
       // loop header.
-      BitVector::Iterator iterator(&data_->spilled_virtual_registers());
-      for (; !iterator.Done(); iterator.Advance()) {
-        const VirtualRegisterData& vreg_data =
-            VirtualRegisterDataFor(iterator.Current());
+      for (int vreg : data_->spilled_virtual_registers()) {
+        const VirtualRegisterData& vreg_data = VirtualRegisterDataFor(vreg);
         if (vreg_data.HasSpillRange() &&
             vreg_data.spill_range()->IsLiveAt(block->first_instruction_index(),
                                               block)) {
@@ -3385,10 +3383,8 @@ void MidTierSpillSlotAllocator::Allocate(
 
 void AllocateSpillSlots(MidTierRegisterAllocationData* data) {
   ZoneVector<VirtualRegisterData*> spilled(data->allocation_zone());
-  BitVector::Iterator iterator(&data->spilled_virtual_registers());
-  for (; !iterator.Done(); iterator.Advance()) {
-    VirtualRegisterData& vreg_data =
-        data->VirtualRegisterDataFor(iterator.Current());
+  for (int vreg : data->spilled_virtual_registers()) {
+    VirtualRegisterData& vreg_data = data->VirtualRegisterDataFor(vreg);
     if (vreg_data.HasPendingSpillOperand()) {
       spilled.push_back(&vreg_data);
     }
@@ -3452,10 +3448,8 @@ void MidTierReferenceMapPopulator::RecordReferences(
 
 void PopulateReferenceMaps(MidTierRegisterAllocationData* data) {
   MidTierReferenceMapPopulator populator(data);
-  BitVector::Iterator iterator(&data->spilled_virtual_registers());
-  for (; !iterator.Done(); iterator.Advance()) {
-    populator.RecordReferences(
-        data->VirtualRegisterDataFor(iterator.Current()));
+  for (int vreg : data->spilled_virtual_registers()) {
+    populator.RecordReferences(data->VirtualRegisterDataFor(vreg));
   }
 }
 

src/compiler/backend/register-allocator.cc

@@ -1404,10 +1404,8 @@ TopTierRegisterAllocationData::GetPhiMapValueFor(TopLevelLiveRange* top_range) {
 
 bool TopTierRegisterAllocationData::ExistsUseWithoutDefinition() {
   bool found = false;
-  BitVector::Iterator iterator(live_in_sets()[0]);
-  while (!iterator.Done()) {
+  for (int operand_index : *live_in_sets()[0]) {
     found = true;
-    int operand_index = iterator.Current();
     PrintF("Register allocator error: live v%d reached first block.\n",
            operand_index);
     LiveRange* range = GetOrCreateLiveRangeFor(operand_index);
@@ -1417,7 +1415,6 @@ bool TopTierRegisterAllocationData::ExistsUseWithoutDefinition() {
     } else {
       PrintF("  (function: %s)\n", debug_name());
     }
-    iterator.Advance();
   }
   return found;
 }
@@ -1895,13 +1892,10 @@ void LiveRangeBuilder::AddInitialIntervals(const InstructionBlock* block,
   LifetimePosition end = LifetimePosition::InstructionFromInstructionIndex(
                              block->last_instruction_index())
                              .NextStart();
-  BitVector::Iterator iterator(live_out);
-  while (!iterator.Done()) {
-    int operand_index = iterator.Current();
+  for (int operand_index : *live_out) {
     TopLevelLiveRange* range = data()->GetOrCreateLiveRangeFor(operand_index);
     range->AddUseInterval(start, end, allocation_zone(),
                           data()->is_trace_alloc());
-    iterator.Advance();
   }
 }
 
@@ -2401,18 +2395,15 @@ void LiveRangeBuilder::ProcessLoopHeader(const InstructionBlock* block,
   DCHECK(block->IsLoopHeader());
   // Add a live range stretching from the first loop instruction to the last
   // for each value live on entry to the header.
-  BitVector::Iterator iterator(live);
   LifetimePosition start = LifetimePosition::GapFromInstructionIndex(
       block->first_instruction_index());
   LifetimePosition end = LifetimePosition::GapFromInstructionIndex(
                              code()->LastLoopInstructionIndex(block))
                              .NextFullStart();
-  while (!iterator.Done()) {
-    int operand_index = iterator.Current();
+  for (int operand_index : *live) {
     TopLevelLiveRange* range = data()->GetOrCreateLiveRangeFor(operand_index);
     range->EnsureInterval(start, end, allocation_zone(),
                           data()->is_trace_alloc());
-    iterator.Advance();
   }
   // Insert all values into the live in sets of all blocks in the loop.
   for (int i = block->rpo_number().ToInt() + 1; i < block->loop_end().ToInt();
@@ -4769,10 +4760,11 @@ void LiveRangeConnector::ResolveControlFlow(Zone* local_zone) {
   for (const InstructionBlock* block : code()->instruction_blocks()) {
     if (CanEagerlyResolveControlFlow(block)) continue;
     BitVector* live = live_in_sets[block->rpo_number().ToInt()];
-    BitVector::Iterator iterator(live);
-    while (!iterator.Done()) {
+    auto it = live->begin();
+    auto end = live->end();
+    while (it != end) {
       data()->tick_counter()->TickAndMaybeEnterSafepoint();
-      int vreg = iterator.Current();
+      int vreg = *it;
       LiveRangeBoundArray* array = finder.ArrayFor(vreg);
       for (const RpoNumber& pred : block->predecessors()) {
         FindResult result;
@@ -4835,7 +4827,7 @@ void LiveRangeConnector::ResolveControlFlow(Zone* local_zone) {
                 move_loc != -1,
             code()->GetInstructionBlock(move_loc)->IsDeferred());
       }
-      iterator.Advance();
+      ++it;
     }
   }
 
@@ -5013,10 +5005,8 @@ void LiveRangeConnector::CommitSpillsInDeferredBlocks(
 
   ZoneQueue<int> worklist(temp_zone);
 
-  for (BitVector::Iterator iterator(
-           range->GetListOfBlocksRequiringSpillOperands(data()));
-       !iterator.Done(); iterator.Advance()) {
-    worklist.push(iterator.Current());
+  for (int block_id : *range->GetListOfBlocksRequiringSpillOperands(data())) {
+    worklist.push(block_id);
   }
 
   ZoneSet<std::pair<RpoNumber, int>> done_moves(temp_zone);

src/compiler/verifier.cc

@@ -2080,9 +2080,8 @@ void ScheduleVerifier::Run(Schedule* schedule) {
       if (idom == nullptr) continue;
       BitVector* block_doms = dominators[block->id().ToSize()];
 
-      for (BitVector::Iterator it(block_doms); !it.Done(); it.Advance()) {
-        BasicBlock* dom =
-            schedule->GetBlockById(BasicBlock::Id::FromInt(it.Current()));
+      for (int id : *block_doms) {
+        BasicBlock* dom = schedule->GetBlockById(BasicBlock::Id::FromInt(id));
         if (dom != idom &&
             !dominators[idom->id().ToSize()]->Contains(dom->id().ToInt())) {
           FATAL("Block B%d is not immediately dominated by B%d",

src/utils/bit-vector.h

@@ -24,19 +24,7 @@ class V8_EXPORT_PRIVATE BitVector : public ZoneObject {
   // Iterator for the elements of this BitVector.
   class Iterator {
    public:
-    explicit Iterator(BitVector* target)
-        : target_(target),
-          current_index_(0),
-          current_value_(target->is_inline() ? target->data_.inline_
-                                             : target->data_.ptr_[0]),
-          current_(-1) {
-      Advance();
-    }
-    ~Iterator() = default;
-
-    bool Done() const { return current_index_ >= target_->data_length_; }
-
-    V8_EXPORT_PRIVATE inline void Advance() {
+    V8_EXPORT_PRIVATE inline void operator++() {
       current_++;
 
       // Skip zeroed words.
@@ -57,15 +45,46 @@ class V8_EXPORT_PRIVATE BitVector : public ZoneObject {
       current_value_ >>= 1;
     }
 
-    int Current() const {
+    int operator*() const {
       DCHECK(!Done());
       return current_;
     }
 
+    bool operator!=(const Iterator& other) const {
+      // "other" is required to be the end sentinel value, to avoid us needing
+      // to compare exact "current" values.
+      DCHECK(other.Done());
+      DCHECK_EQ(target_, other.target_);
+      return current_index_ != other.current_index_;
+    }
+
    private:
-    BitVector* target_;
-    int current_index_;
+    static constexpr struct StartTag {
+    } kStartTag = {};
+    static constexpr struct EndTag {
+    } kEndTag = {};
+
+    explicit Iterator(const BitVector* target, StartTag)
+        : target_(target),
+          current_value_(target->is_inline() ? target->data_.inline_
+                                             : target->data_.ptr_[0]),
+          current_index_(0),
+          current_(-1) {
+      ++(*this);
+    }
+    explicit Iterator(const BitVector* target, EndTag)
+        : target_(target),
+          current_value_(0),
+          current_index_(target->data_length_),
+          current_(-1) {
+      DCHECK(Done());
+    }
+
+    bool Done() const { return current_index_ >= target_->data_length_; }
+
+    const BitVector* target_;
     uintptr_t current_value_;
+    int current_index_;
     int current_;
 
     friend class BitVector;
@@ -295,6 +314,10 @@ class V8_EXPORT_PRIVATE BitVector : public ZoneObject {
 
   int length() const { return length_; }
 
+  Iterator begin() const { return Iterator(this, Iterator::kStartTag); }
+
+  Iterator end() const { return Iterator(this, Iterator::kEndTag); }
+
 #ifdef DEBUG
   void Print() const;
 #endif
@@ -311,63 +334,36 @@ class V8_EXPORT_PRIVATE BitVector : public ZoneObject {
 
 class GrowableBitVector {
  public:
-  class Iterator {
-   public:
-    Iterator(const GrowableBitVector* target, Zone* zone)
-        : it_(target->bits_ == nullptr ? zone->New<BitVector>(1, zone)
-                                       : target->bits_) {}
-    bool Done() const { return it_.Done(); }
-    void Advance() { it_.Advance(); }
-    int Current() const { return it_.Current(); }
-
-   private:
-    BitVector::Iterator it_;
-  };
-
-  GrowableBitVector() : bits_(nullptr) {}
-  GrowableBitVector(int length, Zone* zone)
-      : bits_(zone->New<BitVector>(length, zone)) {}
+  GrowableBitVector() : bits_() {}
+  GrowableBitVector(int length, Zone* zone) : bits_(length, zone) {}
 
   bool Contains(int value) const {
     if (!InBitsRange(value)) return false;
-    return bits_->Contains(value);
+    return bits_.Contains(value);
   }
 
   void Add(int value, Zone* zone) {
     EnsureCapacity(value, zone);
-    bits_->Add(value);
+    bits_.Add(value);
   }
 
-  void Union(const GrowableBitVector& other, Zone* zone) {
-    for (Iterator it(&other, zone); !it.Done(); it.Advance()) {
-      Add(it.Current(), zone);
-    }
-  }
-
-  void Clear() {
-    if (bits_ != nullptr) bits_->Clear();
-  }
+  void Clear() { bits_.Clear(); }
 
  private:
   static const int kInitialLength = 1024;
 
-  bool InBitsRange(int value) const {
-    return bits_ != nullptr && bits_->length() > value;
-  }
+  bool InBitsRange(int value) const { return bits_.length() > value; }
 
   void EnsureCapacity(int value, Zone* zone) {
     if (InBitsRange(value)) return;
-    int new_length = bits_ == nullptr ? kInitialLength : bits_->length();
+    int new_length =
+        base::bits::RoundUpToPowerOfTwo32(static_cast<uint32_t>(value));
+    new_length = std::min(new_length, kInitialLength);
     while (new_length <= value) new_length *= 2;
-
-    if (bits_ == nullptr) {
-      bits_ = zone->New<BitVector>(new_length, zone);
-    } else {
-      bits_->Resize(new_length, zone);
-    }
+    bits_.Resize(new_length, zone);
   }
 
-  BitVector* bits_;
+  BitVector bits_;
 };
 
 }  // namespace internal

test/cctest/test-bit-vector.cc

@@ -59,16 +59,21 @@ TEST(BitVector) {
     v.Add(30);
     v.Add(31);
     v.Add(33);
-    BitVector::Iterator iter(&v);
-    CHECK_EQ(27, iter.Current());
-    iter.Advance();
-    CHECK_EQ(30, iter.Current());
-    iter.Advance();
-    CHECK_EQ(31, iter.Current());
-    iter.Advance();
-    CHECK_EQ(33, iter.Current());
-    iter.Advance();
-    CHECK(iter.Done());
+    BitVector::Iterator iter = v.begin();
+    BitVector::Iterator end = v.end();
+    CHECK_NE(iter, end);
+    CHECK_EQ(27, *iter);
+    ++iter;
+    CHECK_NE(iter, end);
+    CHECK_EQ(30, *iter);
+    ++iter;
+    CHECK_NE(iter, end);
+    CHECK_EQ(31, *iter);
+    ++iter;
+    CHECK_NE(iter, end);
+    CHECK_EQ(33, *iter);
+    ++iter;
+    CHECK(!(iter != end));
   }
 
   {