From 0d30a4d479b4c5ac931610d019b053815e943cca Mon Sep 17 00:00:00 2001
From: Jannis Harder <me@jix.one>
Date: Mon, 22 Apr 2024 13:26:17 +0200
Subject: [PATCH 1/2] rtlil: Add packed `extract` implementation for `SigSpec`

Previously `extract` on a `SigSpec` would always unpack it. Since a
significant amount of `SigSpec`s have one or few chunks, it's worth
having a dedicated implementation.

This is especially true, since the RTLIL frontend calls into this for
every `wire [lhs:rhs]` slice, making this `extract` take up 40% when
profiling `read_rtlil` with one of the largest coarse grained RTLIL
designs I had on hand.

With this change the `read_rtlil` profile looks like I would expect it
to look like, but I noticed that a lot of the other core RTLIL methods
also are a bit too eager with unpacking or implementing
`SigChunk`/`Const` overloads that just convert to a single chunk
`SigSpec` and forward to the implementation for that, when a direct
implementation would avoid temporary std::vector allocations. While not
relevant for `read_rtlil`, to me it looks like there might be a few easy
overall performance gains to be had by addressing this more generally.
---
 kernel/rtlil.cc | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc
index 8781b6a8946..bafafb57b9b 100644
--- a/kernel/rtlil.cc
+++ b/kernel/rtlil.cc
@@ -4435,8 +4435,39 @@ RTLIL::SigSpec RTLIL::SigSpec::extract(int offset, int length) const
 	log_assert(offset >= 0);
 	log_assert(length >= 0);
 	log_assert(offset + length <= width_);
-	unpack();
+
 	cover("kernel.rtlil.sigspec.extract_pos");
+
+	if (packed())
+	{
+		if (chunks_.size() == 1)
+			return chunks_[0].extract(offset, length);
+
+		SigSpec extracted;
+		int end = offset + length;
+		int chunk_end = 0;
+
+		for (auto const &chunk : chunks_)
+		{
+			int chunk_begin = chunk_end;
+			chunk_end += chunk.width;
+			int extract_begin = std::max(chunk_begin, offset);
+			int extract_end = std::min(chunk_end, end);
+			if (extract_begin >= extract_end)
+				continue;
+			int extract_offset = extract_begin - chunk_begin;
+			int extract_len = extract_end - extract_begin;
+			if (extract_offset == 0 && extract_len == chunk.width)
+				extracted.chunks_.emplace_back(chunk);
+			else
+				extracted.chunks_.emplace_back(
+					chunk.extract(extract_offset, extract_len));
+		}
+
+		extracted.width_ = length;
+		return extracted;
+	}
+
 	return std::vector<RTLIL::SigBit>(bits_.begin() + offset, bits_.begin() + offset + length);
 }
 

From 178eceb32d9e6d5e36077347ac452f6e10ae7aab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Povi=C5=A1er?= <povik@cutebit.org>
Date: Mon, 22 Apr 2024 16:23:51 +0200
Subject: [PATCH 2/2] rtlil: Replace the packed `SigSpec::extract` impl

---
 kernel/rtlil.cc | 46 +++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc
index bafafb57b9b..a6aebaa4258 100644
--- a/kernel/rtlil.cc
+++ b/kernel/rtlil.cc
@@ -4438,37 +4438,33 @@ RTLIL::SigSpec RTLIL::SigSpec::extract(int offset, int length) const
 
 	cover("kernel.rtlil.sigspec.extract_pos");
 
-	if (packed())
-	{
-		if (chunks_.size() == 1)
-			return chunks_[0].extract(offset, length);
-
+	if (packed()) {
 		SigSpec extracted;
-		int end = offset + length;
-		int chunk_end = 0;
+		extracted.width_ = length;
 
-		for (auto const &chunk : chunks_)
-		{
-			int chunk_begin = chunk_end;
-			chunk_end += chunk.width;
-			int extract_begin = std::max(chunk_begin, offset);
-			int extract_end = std::min(chunk_end, end);
-			if (extract_begin >= extract_end)
-				continue;
-			int extract_offset = extract_begin - chunk_begin;
-			int extract_len = extract_end - extract_begin;
-			if (extract_offset == 0 && extract_len == chunk.width)
-				extracted.chunks_.emplace_back(chunk);
-			else
-				extracted.chunks_.emplace_back(
-					chunk.extract(extract_offset, extract_len));
+		auto it = chunks_.begin();
+		for (; offset; offset -= it->width, it++) {
+			if (offset < it->width) {
+				int chunk_length = min(it->width - offset, length);
+				extracted.chunks_.emplace_back(it->extract(offset, chunk_length));
+				length -= chunk_length;
+				it++;
+				break;
+			}
+		}
+		for (; length; length -= it->width, it++) {
+			if (length >= it->width) {
+				extracted.chunks_.emplace_back(*it);
+			} else {
+				extracted.chunks_.emplace_back(it->extract(0, length));
+				break;
+			}
 		}
 
-		extracted.width_ = length;
 		return extracted;
+	} else {
+		return std::vector<RTLIL::SigBit>(bits_.begin() + offset, bits_.begin() + offset + length);
 	}
-
-	return std::vector<RTLIL::SigBit>(bits_.begin() + offset, bits_.begin() + offset + length);
 }
 
 void RTLIL::SigSpec::append(const RTLIL::SigSpec &signal)