Make SigSpecConstIterator iterate over SigSpec without unpacking

To make this efficient, the iterator keeps `chunk_index_hint` pointing to
the current chunk. This is only a hint, since it is not possible to maintain
its validity if the `SigSpec` representation is temporarily changed to unpacked.
In that case we have to resync using binary search.

kernel/rtlil.cc

@@ -4447,6 +4447,23 @@ bool RTLIL::SigChunk::operator !=(const RTLIL::SigChunk &other) const
 	return true;
 }
 
+const RTLIL::SigChunk &RTLIL::SigSpecConstIterator::find_chunk()
+{
+	int low = 0;
+	int high = GetSize(sig_p->chunks_);
+	while (high - low >= 2) {
+		int mid = (low + high) / 2;
+		if (sig_p->chunks_[mid].offset_in_sigspec <= bit_index)
+			low = mid;
+		else
+			high = mid;
+	}
+	chunk_index_hint = low;
+	const RTLIL::SigChunk &chunk = sig_p->chunks_[chunk_index_hint];
+	log_assert(chunk.offset_in_sigspec <= bit_index && bit_index < chunk.offset_in_sigspec + chunk.width);
+	return chunk;
+}
+
 RTLIL::SigSpec::SigSpec(std::initializer_list<RTLIL::SigSpec> parts)
 {
 	cover("kernel.rtlil.sigspec.init.list");

kernel/rtlil.h

@@ -1217,18 +1217,29 @@ struct RTLIL::SigSpecIterator
 struct RTLIL::SigSpecConstIterator
 {
 	typedef std::input_iterator_tag iterator_category;
-	typedef RTLIL::SigBit value_type;
+	typedef const RTLIL::SigBit& value_type;
 	typedef ptrdiff_t difference_type;
 	typedef RTLIL::SigBit* pointer;
 	typedef RTLIL::SigBit& reference;
 
 	const RTLIL::SigSpec *sig_p;
-	int index;
+	SigBit bit;
+	int bit_index;
+	// Index of chunk containing bit_index, or GetSize(chunks_) if bit_index == width_.
+	// or 0 if *sig_p is not packed. This is a hint and may be incorrect in unusual
+	// circumstances, e.g. when the SigSpec changes from unpacked to packed
+	// during iteration.
+	int chunk_index_hint;
 
-	inline const RTLIL::SigBit &operator*() const;
-	inline bool operator!=(const RTLIL::SigSpecConstIterator &other) const { return index != other.index; }
-	inline bool operator==(const RTLIL::SigSpecIterator &other) const { return index == other.index; }
-	inline void operator++() { index++; }
+	inline const RTLIL::SigBit &operator*();
+	inline bool operator!=(const RTLIL::SigSpecConstIterator &other) const { return !(*this == other); }
+	inline bool operator==(const RTLIL::SigSpecConstIterator &other) const { return bit_index == other.bit_index; }
+	inline void operator++();
+
+private:
+	// Must be called when sig_p is packed and `bit_index` is in range. Finds the chunk containing `bit_index`
+	// and sets chunk_index_hint.
+	const RTLIL::SigChunk &find_chunk();
 };
 
 struct RTLIL::SigSpec
@@ -1255,6 +1266,7 @@ private:
 	// Only used by Module::remove(const pool<Wire*> &wires)
 	// but cannot be more specific as it isn't yet declared
 	friend struct RTLIL::Module;
+	friend struct RTLIL::SigSpecConstIterator;
 
 public:
 	SigSpec() : width_(0), hash_(0) {}
@@ -1292,8 +1304,20 @@ public:
 	inline RTLIL::SigSpecIterator begin() { RTLIL::SigSpecIterator it; it.sig_p = this; it.index = 0; return it; }
 	inline RTLIL::SigSpecIterator end() { RTLIL::SigSpecIterator it; it.sig_p = this; it.index = width_; return it; }
 
-	inline RTLIL::SigSpecConstIterator begin() const { RTLIL::SigSpecConstIterator it; it.sig_p = this; it.index = 0; return it; }
-	inline RTLIL::SigSpecConstIterator end() const { RTLIL::SigSpecConstIterator it; it.sig_p = this; it.index = width_; return it; }
+	inline RTLIL::SigSpecConstIterator begin() const {
+		RTLIL::SigSpecConstIterator it;
+		it.sig_p = this;
+		it.bit_index = 0;
+		it.chunk_index_hint = 0;
+		return it;
+	}
+	inline RTLIL::SigSpecConstIterator end() const {
+		RTLIL::SigSpecConstIterator it;
+		it.sig_p = this;
+		it.bit_index = width_;
+		it.chunk_index_hint = GetSize(chunks_);
+		return it;
+	}
 
 	void sort();
 	void sort_and_unify();
@@ -2313,8 +2337,30 @@ inline RTLIL::SigBit &RTLIL::SigSpecIterator::operator*() const {
 	return (*sig_p)[index];
 }
 
-inline const RTLIL::SigBit &RTLIL::SigSpecConstIterator::operator*() const {
-	return (*sig_p)[index];
+inline const RTLIL::SigBit &RTLIL::SigSpecConstIterator::operator*() {
+	if (!sig_p->packed())
+		return sig_p->bits_.at(bit_index);
+	if (chunk_index_hint < GetSize(sig_p->chunks_)) {
+		const SigChunk &chunk = sig_p->chunks_[chunk_index_hint];
+		if (chunk.offset_in_sigspec <= bit_index && bit_index < chunk.offset_in_sigspec + chunk.width) {
+			bit = chunk[bit_index - chunk.offset_in_sigspec];
+			return bit;
+		}
+	}
+	const SigChunk &chunk = find_chunk();
+	bit = chunk[bit_index - chunk.offset_in_sigspec];
+	return bit;
+}
+
+inline void RTLIL::SigSpecConstIterator::operator++() {
+	++bit_index;
+	if (!sig_p->packed())
+		return;
+	if (chunk_index_hint < GetSize(sig_p->chunks_)) {
+		const SigChunk &chunk = sig_p->chunks_[chunk_index_hint];
+		if (chunk.offset_in_sigspec + chunk.width == bit_index)
+			++chunk_index_hint;
+	}
 }
 
 inline RTLIL::SigBit::SigBit(const RTLIL::SigSpec &sig) {

tests/unit/kernel/rtlilTest.cc

@@ -361,6 +361,38 @@ namespace RTLIL {
 		EXPECT_FALSE(Const().is_onehot(&pos));
 	}
 
+	TEST_F(KernelRtlilTest, SigSpecConstIteratorRepresentationChange) {
+		std::unique_ptr<Module> mod = std::make_unique<Module>();
+		std::vector<Wire*> wires;
+		SigSpec spec;
+		for (int i = 0; i < 10; i++) {
+			wires.push_back(mod->addWire(IdString(stringf("\\test%d", i)), 10));
+			spec.append(wires.back());
+		}
+
+		const SigSpec &const_spec = spec;
+		SigSpecConstIterator it = const_spec.begin();
+		for (int i = 0; i < 55; ++i)
+			++it;
+		EXPECT_EQ(*it, SigBit(wires[5], 5));
+
+		// Force unpacking of the spec.
+		EXPECT_EQ(spec[55], SigBit(wires[5], 5));
+		// Make sure the iterator is still OK
+		EXPECT_EQ(*it, SigBit(wires[5], 5));
+
+		// Advance iterator while unpacked
+		for (int i = 0; i < 20; ++i)
+			++it;
+		EXPECT_EQ(*it, SigBit(wires[7], 5));
+
+		// Force packing of the spec.
+		SigSpec other = spec;
+		EXPECT_FALSE(spec < other);
+		// Make sure iterator is still OK
+		EXPECT_EQ(*it, SigBit(wires[7], 5));
+	}
+
 	class WireRtlVsHdlIndexConversionTest :
 		public KernelRtlilTest,
 		public testing::WithParamInterface<std::tuple<bool, int, int>>