From cbb776c62690ddf6fea4e79f48cdc60c11eb3c22 Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Fri, 18 Oct 2024 16:25:15 +0200
Subject: [PATCH 1/7] opt_merge: avoid hashing strings
---
passes/opt/opt_merge.cc | 173 +++++++++++++++++++++-------------------
1 file changed, 90 insertions(+), 83 deletions(-)
diff --git a/passes/opt/opt_merge.cc b/passes/opt/opt_merge.cc
index eb3aa462e..d6ed74433 100644
--- a/passes/opt/opt_merge.cc
+++ b/passes/opt/opt_merge.cc
@@ -26,6 +26,8 @@
#include <stdlib.h>
#include <stdio.h>
#include <set>
+#include <unordered_map>
+#include <array>
USING_YOSYS_NAMESPACE
@@ -42,6 +44,22 @@ struct OptMergeWorker
CellTypes ct;
int total_count;
+ static vector<pair<SigBit, SigSpec>> sorted_pmux_in(const dict<RTLIL::IdString, RTLIL::SigSpec> &conn)
+ {
+ SigSpec sig_s = conn.at(ID::S);
+ SigSpec sig_b = conn.at(ID::B);
+
+ int s_width = GetSize(sig_s);
+ int width = GetSize(sig_b) / s_width;
+
+ vector<pair<SigBit, SigSpec>> sb_pairs;
+ for (int i = 0; i < s_width; i++)
+ sb_pairs.push_back(pair<SigBit, SigSpec>(sig_s[i], sig_b.extract(i*width, width)));
+
+ std::sort(sb_pairs.begin(), sb_pairs.end());
+ return sb_pairs;
+ }
+
static void sort_pmux_conn(dict<RTLIL::IdString, RTLIL::SigSpec> &conn)
{
SigSpec sig_s = conn.at(ID::S);
@@ -65,90 +83,74 @@ struct OptMergeWorker
}
}
- std::string int_to_hash_string(unsigned int v)
+ Hasher hash_cell_inputs(const RTLIL::Cell *cell, Hasher h)
{
- if (v == 0)
- return "0";
- std::string str = "";
- while (v > 0) {
- str += 'a' + (v & 15);
- v = v >> 4;
- }
- return str;
- }
-
- uint64_t hash_cell_parameters_and_connections(const RTLIL::Cell *cell)
- {
- vector<string> hash_conn_strings;
- std::string hash_string = cell->type.str() + "\n";
-
- const dict<RTLIL::IdString, RTLIL::SigSpec> *conn = &cell->connections();
- dict<RTLIL::IdString, RTLIL::SigSpec> alt_conn;
-
+ // TODO: when implemented, use celltypes to match:
+ // (builtin || stdcell) && (unary || binary) && symmetrical
if (cell->type.in(ID($and), ID($or), ID($xor), ID($xnor), ID($add), ID($mul),
ID($logic_and), ID($logic_or), ID($_AND_), ID($_OR_), ID($_XOR_))) {
- alt_conn = *conn;
- if (assign_map(alt_conn.at(ID::A)) < assign_map(alt_conn.at(ID::B))) {
- alt_conn[ID::A] = conn->at(ID::B);
- alt_conn[ID::B] = conn->at(ID::A);
+ std::array<RTLIL::SigSpec, 2> inputs = {
+ assign_map(cell->getPort(ID::A)),
+ assign_map(cell->getPort(ID::B))
+ };
+ std::sort(inputs.begin(), inputs.end());
+ h = hash_ops<std::array<RTLIL::SigSpec, 2>>::hash_acc(inputs, h);
+ h = assign_map(cell->getPort(ID::Y)).hash_acc(h);
+ } else if (cell->type.in(ID($reduce_xor), ID($reduce_xnor))) {
+ SigSpec a = assign_map(cell->getPort(ID::A));
+ a.sort();
+ h = a.hash_acc(h);
+ } else if (cell->type.in(ID($reduce_and), ID($reduce_or), ID($reduce_bool))) {
+ SigSpec a = assign_map(cell->getPort(ID::A));
+ a.sort_and_unify();
+ h = a.hash_acc(h);
+ } else if (cell->type == ID($pmux)) {
+ dict<RTLIL::IdString, RTLIL::SigSpec> conn = cell->connections();
+ assign_map.apply(conn.at(ID::A));
+ assign_map.apply(conn.at(ID::B));
+ assign_map.apply(conn.at(ID::S));
+ for (const auto& [s_bit, b_chunk] : sorted_pmux_in(conn)) {
+ h = s_bit.hash_acc(h);
+ h = b_chunk.hash_acc(h);
}
- conn = &alt_conn;
- } else
- if (cell->type.in(ID($reduce_xor), ID($reduce_xnor))) {
- alt_conn = *conn;
- assign_map.apply(alt_conn.at(ID::A));
- alt_conn.at(ID::A).sort();
- conn = &alt_conn;
- } else
- if (cell->type.in(ID($reduce_and), ID($reduce_or), ID($reduce_bool))) {
- alt_conn = *conn;
- assign_map.apply(alt_conn.at(ID::A));
- alt_conn.at(ID::A).sort_and_unify();
- conn = &alt_conn;
- } else
- if (cell->type == ID($pmux)) {
- alt_conn = *conn;
- assign_map.apply(alt_conn.at(ID::A));
- assign_map.apply(alt_conn.at(ID::B));
- assign_map.apply(alt_conn.at(ID::S));
- sort_pmux_conn(alt_conn);
- conn = &alt_conn;
- }
-
- for (auto &it : *conn) {
- RTLIL::SigSpec sig;
- if (cell->output(it.first)) {
- if (it.first == ID::Q && RTLIL::builtin_ff_cell_types().count(cell->type)) {
- // For the 'Q' output of state elements,
- // use its (* init *) attribute value
- sig = initvals(it.second);
+ h = assign_map(cell->getPort(ID::A)).hash_acc(h);
+ } else {
+ std::vector<std::pair<IdString, SigSpec>> conns;
+ for (const auto& conn : cell->connections()) {
+ conns.push_back(conn);
+ }
+ std::sort(conns.begin(), conns.end());
+ for (const auto& [port, sig] : conns) {
+ if (!cell->output(port)) {
+ h = port.hash_acc(h);
+ h = assign_map(sig).hash_acc(h);
}
- else
- continue;
}
- else
- sig = assign_map(it.second);
- string s = "C " + it.first.str() + "=";
- for (auto &chunk : sig.chunks()) {
- if (chunk.wire)
- s += "{" + chunk.wire->name.str() + " " +
- int_to_hash_string(chunk.offset) + " " +
- int_to_hash_string(chunk.width) + "}";
- else
- s += RTLIL::Const(chunk.data).as_string();
- }
- hash_conn_strings.push_back(s + "\n");
+
+ if (RTLIL::builtin_ff_cell_types().count(cell->type))
+ h = initvals(cell->getPort(ID::Q)).hash_acc(h);
+
}
+ return h;
+ }
- for (auto &it : cell->parameters)
- hash_conn_strings.push_back("P " + it.first.str() + "=" + it.second.as_string() + "\n");
+ static Hasher hash_cell_parameters(const RTLIL::Cell *cell, Hasher h)
+ {
+ using Paramvec = std::vector<std::pair<IdString, Const>>;
+ Paramvec params;
+ for (const auto& param : cell->parameters) {
+ params.push_back(param);
+ }
+ std::sort(params.begin(), params.end());
+ return hash_ops<Paramvec>::hash_acc(params, h);
+ }
- std::sort(hash_conn_strings.begin(), hash_conn_strings.end());
-
- for (auto it : hash_conn_strings)
- hash_string += it;
-
- return std::hash<std::string>{}(hash_string);
+ Hasher hash_cell_function(const RTLIL::Cell *cell, Hasher h)
+ {
+ h.eat(cell->type);
+ h = hash_cell_inputs(cell, h);
+ h = hash_cell_parameters(cell, h);
+ return h;
}
bool compare_cell_parameters_and_connections(const RTLIL::Cell *cell1, const RTLIL::Cell *cell2)
@@ -255,18 +257,23 @@ struct OptMergeWorker
while (did_something)
{
std::vector<RTLIL::Cell*> cells;
- cells.reserve(module->cells_.size());
- for (auto &it : module->cells_) {
- if (!design->selected(module, it.second))
+ cells.reserve(module->cells().size());
+ for (auto cell : module->cells()) {
+ if (!design->selected(module, cell))
continue;
- if (mode_keepdc && has_dont_care_initval(it.second))
+ if (cell->type.in(ID($meminit), ID($meminit_v2), ID($mem), ID($mem_v2))) {
+ // Ignore those for performance: meminit can have an excessively large port,
+ // mem can have an excessively large parameter holding the init data
continue;
- if (ct.cell_known(it.second->type) || (mode_share_all && it.second->known()))
- cells.push_back(it.second);
+ }
+ if (mode_keepdc && has_dont_care_initval(cell))
+ continue;
+ if (ct.cell_known(cell->type) || (mode_share_all && cell->known()))
+ cells.push_back(cell);
}
did_something = false;
- dict<uint64_t, RTLIL::Cell*> sharemap;
+ dict<Hasher::hash_t, RTLIL::Cell*> sharemap;
for (auto cell : cells)
{
if ((!mode_share_all && !ct.cell_known(cell->type)) || !cell->known())
@@ -275,7 +282,7 @@ struct OptMergeWorker
if (cell->type == ID($scopeinfo))
continue;
- uint64_t hash = hash_cell_parameters_and_connections(cell);
+ Hasher::hash_t hash = hash_cell_function(cell, Hasher()).yield();
auto r = sharemap.insert(std::make_pair(hash, cell));
if (!r.second) {
if (compare_cell_parameters_and_connections(cell, r.first->second)) {
From ffc057a89c76f922c4bd5c13383551fc6f9233a3 Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Fri, 18 Oct 2024 20:39:52 +0200
Subject: [PATCH 2/7] opt_merge: fix the many collisions case
---
passes/opt/opt_merge.cc | 45 ++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/passes/opt/opt_merge.cc b/passes/opt/opt_merge.cc
index d6ed74433..025a6c977 100644
--- a/passes/opt/opt_merge.cc
+++ b/passes/opt/opt_merge.cc
@@ -254,6 +254,11 @@ struct OptMergeWorker
initvals.set(&assign_map, module);
bool did_something = true;
+ bool warned_collisions = false;
+ // A cell may have to go through a lot of collisions if the hash
+ // function is performing poorly, but it's a symptom of something bad
+ // beyond the user's control.
+ int threshold = 1000; // adjust to taste
while (did_something)
{
std::vector<RTLIL::Cell*> cells;
@@ -273,7 +278,10 @@ struct OptMergeWorker
}
did_something = false;
- dict<Hasher::hash_t, RTLIL::Cell*> sharemap;
+ // INVARIANT: All cells associated with the same hash in sharemap
+ // are distinct as far as compare_cell_parameters_and_connections
+ // can tell.
+ std::unordered_multimap<Hasher::hash_t, RTLIL::Cell*> sharemap;
for (auto cell : cells)
{
if ((!mode_share_all && !ct.cell_known(cell->type)) || !cell->known())
@@ -283,21 +291,30 @@ struct OptMergeWorker
continue;
Hasher::hash_t hash = hash_cell_function(cell, Hasher()).yield();
- auto r = sharemap.insert(std::make_pair(hash, cell));
- if (!r.second) {
- if (compare_cell_parameters_and_connections(cell, r.first->second)) {
+ // Get all cells with matching hashes
+ auto matching = sharemap.equal_range(hash);
+ int collisions = 0;
+ bool found = false;
+ for (auto it = matching.first; it != matching.second && !found; ++it) {
+ if (collisions > threshold && !warned_collisions) {
+ log_warning("High hash collision counts. opt_merge runtime may be excessive.\n" \
+ "Please report to maintainers.\n");
+ warned_collisions = true;
+ }
+ auto other_cell = it->second;
+ if (compare_cell_parameters_and_connections(cell, other_cell)) {
+ found = true;
if (cell->has_keep_attr()) {
- if (r.first->second->has_keep_attr())
+ if (other_cell->has_keep_attr())
continue;
- std::swap(r.first->second, cell);
+ std::swap(other_cell, cell);
}
-
did_something = true;
- log_debug(" Cell `%s' is identical to cell `%s'.\n", cell->name.c_str(), r.first->second->name.c_str());
+ log_debug(" Cell `%s' is identical to cell `%s'.\n", cell->name.c_str(), other_cell->name.c_str());
for (auto &it : cell->connections()) {
if (cell->output(it.first)) {
- RTLIL::SigSpec other_sig = r.first->second->getPort(it.first);
+ RTLIL::SigSpec other_sig = other_cell->getPort(it.first);
log_debug(" Redirecting output %s: %s = %s\n", it.first.c_str(),
log_signal(it.second), log_signal(other_sig));
Const init = initvals(other_sig);
@@ -311,8 +328,18 @@ struct OptMergeWorker
log_debug(" Removing %s cell `%s' from module `%s'.\n", cell->type.c_str(), cell->name.c_str(), module->name.c_str());
module->remove(cell);
total_count++;
+ break;
+ } else {
+ collisions++;
+ log_debug(" False hash match: `%s' and `%s'.\n", cell->name.c_str(), other_cell->name.c_str());
}
}
+ if (!did_something) {
+ // This cell isn't represented yet in the sharemap.
+ // Either it's the first of its hash,
+ // or falsely matches all cells in sharemap sharing its hash.
+ sharemap.insert(std::make_pair(hash, cell));
+ }
}
}
From 890374014714532726ceab36d7b4eedb78e119eb Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Tue, 12 Nov 2024 14:59:09 +0100
Subject: [PATCH 3/7] opt_merge: switch to unordered_set
---
passes/opt/opt_merge.cc | 126 +++++++++++++++++++---------------------
1 file changed, 61 insertions(+), 65 deletions(-)
diff --git a/passes/opt/opt_merge.cc b/passes/opt/opt_merge.cc
index 025a6c977..1992aa871 100644
--- a/passes/opt/opt_merge.cc
+++ b/passes/opt/opt_merge.cc
@@ -83,7 +83,7 @@ struct OptMergeWorker
}
}
- Hasher hash_cell_inputs(const RTLIL::Cell *cell, Hasher h)
+ Hasher hash_cell_inputs(const RTLIL::Cell *cell, Hasher h) const
{
// TODO: when implemented, use celltypes to match:
// (builtin || stdcell) && (unary || binary) && symmetrical
@@ -94,26 +94,26 @@ struct OptMergeWorker
assign_map(cell->getPort(ID::B))
};
std::sort(inputs.begin(), inputs.end());
- h = hash_ops<std::array<RTLIL::SigSpec, 2>>::hash_acc(inputs, h);
- h = assign_map(cell->getPort(ID::Y)).hash_acc(h);
+ h = hash_ops<std::array<RTLIL::SigSpec, 2>>::hash_into(inputs, h);
+ h = assign_map(cell->getPort(ID::Y)).hash_into(h);
} else if (cell->type.in(ID($reduce_xor), ID($reduce_xnor))) {
SigSpec a = assign_map(cell->getPort(ID::A));
a.sort();
- h = a.hash_acc(h);
+ h = a.hash_into(h);
} else if (cell->type.in(ID($reduce_and), ID($reduce_or), ID($reduce_bool))) {
SigSpec a = assign_map(cell->getPort(ID::A));
a.sort_and_unify();
- h = a.hash_acc(h);
+ h = a.hash_into(h);
} else if (cell->type == ID($pmux)) {
dict<RTLIL::IdString, RTLIL::SigSpec> conn = cell->connections();
assign_map.apply(conn.at(ID::A));
assign_map.apply(conn.at(ID::B));
assign_map.apply(conn.at(ID::S));
for (const auto& [s_bit, b_chunk] : sorted_pmux_in(conn)) {
- h = s_bit.hash_acc(h);
- h = b_chunk.hash_acc(h);
+ h = s_bit.hash_into(h);
+ h = b_chunk.hash_into(h);
}
- h = assign_map(cell->getPort(ID::A)).hash_acc(h);
+ h = assign_map(cell->getPort(ID::A)).hash_into(h);
} else {
std::vector<std::pair<IdString, SigSpec>> conns;
for (const auto& conn : cell->connections()) {
@@ -122,13 +122,13 @@ struct OptMergeWorker
std::sort(conns.begin(), conns.end());
for (const auto& [port, sig] : conns) {
if (!cell->output(port)) {
- h = port.hash_acc(h);
- h = assign_map(sig).hash_acc(h);
+ h = port.hash_into(h);
+ h = assign_map(sig).hash_into(h);
}
}
if (RTLIL::builtin_ff_cell_types().count(cell->type))
- h = initvals(cell->getPort(ID::Q)).hash_acc(h);
+ h = initvals(cell->getPort(ID::Q)).hash_into(h);
}
return h;
@@ -142,10 +142,10 @@ struct OptMergeWorker
params.push_back(param);
}
std::sort(params.begin(), params.end());
- return hash_ops<Paramvec>::hash_acc(params, h);
+ return hash_ops<Paramvec>::hash_into(params, h);
}
- Hasher hash_cell_function(const RTLIL::Cell *cell, Hasher h)
+ Hasher hash_cell_function(const RTLIL::Cell *cell, Hasher h) const
{
h.eat(cell->type);
h = hash_cell_inputs(cell, h);
@@ -153,7 +153,7 @@ struct OptMergeWorker
return h;
}
- bool compare_cell_parameters_and_connections(const RTLIL::Cell *cell1, const RTLIL::Cell *cell2)
+ bool compare_cell_parameters_and_connections(const RTLIL::Cell *cell1, const RTLIL::Cell *cell2) const
{
log_assert(cell1 != cell2);
if (cell1->type != cell2->type) return false;
@@ -254,11 +254,9 @@ struct OptMergeWorker
initvals.set(&assign_map, module);
bool did_something = true;
- bool warned_collisions = false;
// A cell may have to go through a lot of collisions if the hash
// function is performing poorly, but it's a symptom of something bad
// beyond the user's control.
- int threshold = 1000; // adjust to taste
while (did_something)
{
std::vector<RTLIL::Cell*> cells;
@@ -278,10 +276,29 @@ struct OptMergeWorker
}
did_something = false;
- // INVARIANT: All cells associated with the same hash in sharemap
- // are distinct as far as compare_cell_parameters_and_connections
- // can tell.
- std::unordered_multimap<Hasher::hash_t, RTLIL::Cell*> sharemap;
+
+ // We keep a set of known cells. They're hashed with our hash_cell_function
+ // and compared with our compare_cell_parameters_and_connections.
+ // Both need to capture OptMergeWorker to access initvals
+ struct CellPtrHash {
+ const OptMergeWorker& worker;
+ CellPtrHash(const OptMergeWorker& w) : worker(w) {}
+ std::size_t operator()(const Cell* c) const {
+ return (std::size_t)worker.hash_cell_function(c, Hasher()).yield();
+ }
+ };
+ struct CellPtrEqual {
+ const OptMergeWorker& worker;
+ CellPtrEqual(const OptMergeWorker& w) : worker(w) {}
+ bool operator()(const Cell* lhs, const Cell* rhs) const {
+ return worker.compare_cell_parameters_and_connections(lhs, rhs);
+ }
+ };
+ std::unordered_set<
+ RTLIL::Cell*,
+ CellPtrHash,
+ CellPtrEqual> known_cells (0, CellPtrHash(*this), CellPtrEqual(*this));
+
for (auto cell : cells)
{
if ((!mode_share_all && !ct.cell_known(cell->type)) || !cell->known())
@@ -290,55 +307,34 @@ struct OptMergeWorker
if (cell->type == ID($scopeinfo))
continue;
- Hasher::hash_t hash = hash_cell_function(cell, Hasher()).yield();
- // Get all cells with matching hashes
- auto matching = sharemap.equal_range(hash);
- int collisions = 0;
- bool found = false;
- for (auto it = matching.first; it != matching.second && !found; ++it) {
- if (collisions > threshold && !warned_collisions) {
- log_warning("High hash collision counts. opt_merge runtime may be excessive.\n" \
- "Please report to maintainers.\n");
- warned_collisions = true;
+ auto [cell_in_map, inserted] = known_cells.insert(cell);
+ if (!inserted) {
+ // We've failed to insert since we already have an equivalent cell
+ Cell* other_cell = *cell_in_map;
+ if (cell->has_keep_attr()) {
+ if (other_cell->has_keep_attr())
+ continue;
+ std::swap(other_cell, cell);
}
- auto other_cell = it->second;
- if (compare_cell_parameters_and_connections(cell, other_cell)) {
- found = true;
- if (cell->has_keep_attr()) {
- if (other_cell->has_keep_attr())
- continue;
- std::swap(other_cell, cell);
- }
- did_something = true;
- log_debug(" Cell `%s' is identical to cell `%s'.\n", cell->name.c_str(), other_cell->name.c_str());
- for (auto &it : cell->connections()) {
- if (cell->output(it.first)) {
- RTLIL::SigSpec other_sig = other_cell->getPort(it.first);
- log_debug(" Redirecting output %s: %s = %s\n", it.first.c_str(),
- log_signal(it.second), log_signal(other_sig));
- Const init = initvals(other_sig);
- initvals.remove_init(it.second);
- initvals.remove_init(other_sig);
- module->connect(RTLIL::SigSig(it.second, other_sig));
- assign_map.add(it.second, other_sig);
- initvals.set_init(other_sig, init);
- }
+ did_something = true;
+ log_debug(" Cell `%s' is identical to cell `%s'.\n", cell->name.c_str(), other_cell->name.c_str());
+ for (auto &it : cell->connections()) {
+ if (cell->output(it.first)) {
+ RTLIL::SigSpec other_sig = other_cell->getPort(it.first);
+ log_debug(" Redirecting output %s: %s = %s\n", it.first.c_str(),
+ log_signal(it.second), log_signal(other_sig));
+ Const init = initvals(other_sig);
+ initvals.remove_init(it.second);
+ initvals.remove_init(other_sig);
+ module->connect(RTLIL::SigSig(it.second, other_sig));
+ assign_map.add(it.second, other_sig);
+ initvals.set_init(other_sig, init);
}
- log_debug(" Removing %s cell `%s' from module `%s'.\n", cell->type.c_str(), cell->name.c_str(), module->name.c_str());
- module->remove(cell);
- total_count++;
- break;
- } else {
- collisions++;
- log_debug(" False hash match: `%s' and `%s'.\n", cell->name.c_str(), other_cell->name.c_str());
}
- }
- if (!did_something) {
- // This cell isn't represented yet in the sharemap.
- // Either it's the first of its hash,
- // or falsely matches all cells in sharemap sharing its hash.
- sharemap.insert(std::make_pair(hash, cell));
+ log_debug(" Removing %s cell `%s' from module `%s'.\n", cell->type.c_str(), cell->name.c_str(), module->name.c_str());
+ module->remove(cell);
+ total_count++;
}
}
}
From 176faae7c94b0dea13052b17d4f210cb19b2d7ba Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Mon, 10 Mar 2025 11:43:13 +0100
Subject: [PATCH 4/7] opt_merge: fix trivial binary regression
---
passes/opt/opt_merge.cc | 1 -
tests/opt/opt_merge_basic.ys | 13 +++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 tests/opt/opt_merge_basic.ys
diff --git a/passes/opt/opt_merge.cc b/passes/opt/opt_merge.cc
index 1992aa871..745b27e87 100644
--- a/passes/opt/opt_merge.cc
+++ b/passes/opt/opt_merge.cc
@@ -95,7 +95,6 @@ struct OptMergeWorker
};
std::sort(inputs.begin(), inputs.end());
h = hash_ops<std::array<RTLIL::SigSpec, 2>>::hash_into(inputs, h);
- h = assign_map(cell->getPort(ID::Y)).hash_into(h);
} else if (cell->type.in(ID($reduce_xor), ID($reduce_xnor))) {
SigSpec a = assign_map(cell->getPort(ID::A));
a.sort();
diff --git a/tests/opt/opt_merge_basic.ys b/tests/opt/opt_merge_basic.ys
new file mode 100644
index 000000000..e6336f648
--- /dev/null
+++ b/tests/opt/opt_merge_basic.ys
@@ -0,0 +1,13 @@
+read_verilog -icells <<EOT
+module top(A, B, X, Y);
+input [8:0] A, B;
+output [8:0] X, Y;
+assign X = A + B;
+assign Y = A + B;
+endmodule
+EOT
+
+select -assert-count 2 t:$add
+equiv_opt -assert opt_merge
+design -load postopt
+select -assert-count 1 t:$add
From ae7a97cc2d8bb2e92c96b08f83de4fc57e12fe74 Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Mon, 10 Mar 2025 12:30:00 +0100
Subject: [PATCH 5/7] opt_merge: test some unary cells
---
tests/opt/opt_merge_basic.ys | 69 +++++++++++++++++++++++++++++++++---
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/tests/opt/opt_merge_basic.ys b/tests/opt/opt_merge_basic.ys
index e6336f648..082fdb0b8 100644
--- a/tests/opt/opt_merge_basic.ys
+++ b/tests/opt/opt_merge_basic.ys
@@ -1,13 +1,74 @@
-read_verilog -icells <<EOT
+read_verilog <<EOT
module top(A, B, X, Y);
-input [8:0] A, B;
-output [8:0] X, Y;
+input [7:0] A, B;
+output [7:0] X, Y;
assign X = A + B;
assign Y = A + B;
endmodule
EOT
-
+# Most basic case
+# Binary
select -assert-count 2 t:$add
equiv_opt -assert opt_merge
design -load postopt
select -assert-count 1 t:$add
+
+design -reset
+read_verilog <<EOT
+module top(A, B, C, X, Y);
+input [7:0] A, B, C;
+output [7:0] X, Y;
+assign X = A + B;
+assign Y = A + C;
+endmodule
+EOT
+# Reject on a different input
+select -assert-count 2 t:$add
+opt_merge
+select -assert-count 2 t:$add
+
+design -reset
+read_verilog <<EOT
+module top(A, X, Y);
+input [7:0] A;
+output X, Y;
+assign X = ^A;
+assign Y = ^A;
+endmodule
+EOT
+# Unary
+select -assert-count 2 t:$reduce_xor
+dump
+opt_merge
+select -assert-count 1 t:$reduce_xor
+
+design -reset
+read_verilog -icells <<EOT
+module top(A, B, X, Y);
+input [7:0] A;
+input [7:0] B;
+output X, Y;
+ \$reduce_or #(
+ .A_SIGNED(32'd0),
+ .A_WIDTH(32'd16),
+ .Y_WIDTH(32'd1),
+ ) one (
+ .A({A, B}), // <- look here
+ .Y(X)
+ );
+ \$reduce_or #(
+ .A_SIGNED(32'd0),
+ .A_WIDTH(32'd16),
+ .Y_WIDTH(32'd1),
+ ) other (
+ .A({B, A}), // <- look here
+ .Y(Y)
+ );
+endmodule
+EOT
+# Unary
+opt_expr
+select -assert-count 2 t:$reduce_or
+equiv_opt -assert opt_merge
+design -load postopt
+select -assert-count 1 t:$reduce_or
From 33bfc9d19cc34ee25df127eab1d2cdad057a1dad Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Mon, 10 Mar 2025 12:56:21 +0100
Subject: [PATCH 6/7] opt_merge: test more kinds of cells
---
tests/opt/opt_merge_basic.ys | 104 +++++++++++++++++++++++++++++++++--
1 file changed, 98 insertions(+), 6 deletions(-)
diff --git a/tests/opt/opt_merge_basic.ys b/tests/opt/opt_merge_basic.ys
index 082fdb0b8..9de320aaa 100644
--- a/tests/opt/opt_merge_basic.ys
+++ b/tests/opt/opt_merge_basic.ys
@@ -19,7 +19,7 @@ module top(A, B, C, X, Y);
input [7:0] A, B, C;
output [7:0] X, Y;
assign X = A + B;
-assign Y = A + C;
+assign Y = A + C; // <- look here
endmodule
EOT
# Reject on a different input
@@ -45,10 +45,9 @@ select -assert-count 1 t:$reduce_xor
design -reset
read_verilog -icells <<EOT
module top(A, B, X, Y);
-input [7:0] A;
-input [7:0] B;
+input [7:0] A, B;
output X, Y;
- \$reduce_or #(
+ \$reduce_xor #(
.A_SIGNED(32'd0),
.A_WIDTH(32'd16),
.Y_WIDTH(32'd1),
@@ -56,7 +55,7 @@ output X, Y;
.A({A, B}), // <- look here
.Y(X)
);
- \$reduce_or #(
+ \$reduce_xor #(
.A_SIGNED(32'd0),
.A_WIDTH(32'd16),
.Y_WIDTH(32'd1),
@@ -66,9 +65,102 @@ output X, Y;
);
endmodule
EOT
-# Unary
+# Unary is sorted
+opt_expr
+select -assert-count 2 t:$reduce_xor
+equiv_opt -assert opt_merge
+design -load postopt
+select -assert-count 1 t:$reduce_xor
+
+design -reset
+read_verilog -icells <<EOT
+module top(A, B, X, Y);
+input [7:0] A, B;
+output X, Y;
+ \$reduce_or #(
+ .A_SIGNED(32'd0),
+ .A_WIDTH(32'd24),
+ .Y_WIDTH(32'd1),
+ ) one (
+ .A({A, B, B}), // <- look here
+ .Y(X)
+ );
+ \$reduce_or #(
+ .A_SIGNED(32'd0),
+ .A_WIDTH(32'd24),
+ .Y_WIDTH(32'd1),
+ ) other (
+ .A({A, A, B}), // <- look here
+ .Y(Y)
+ );
+endmodule
+EOT
+# Unary is unified when valid
opt_expr
select -assert-count 2 t:$reduce_or
equiv_opt -assert opt_merge
design -load postopt
select -assert-count 1 t:$reduce_or
+
+design -reset
+read_verilog -icells <<EOT
+module top(A, B, X, Y);
+input [7:0] A, B;
+output X, Y;
+ \$reduce_xor #(
+ .A_SIGNED(32'd0),
+ .A_WIDTH(32'd24),
+ .Y_WIDTH(32'd1),
+ ) one (
+ .A({A, B, B}), // <- look here
+ .Y(X)
+ );
+ \$reduce_xor #(
+ .A_SIGNED(32'd0),
+ .A_WIDTH(32'd24),
+ .Y_WIDTH(32'd1),
+ ) other (
+ .A({A, A, B}), // <- look here
+ .Y(Y)
+ );
+endmodule
+EOT
+# Unary isn't unified when that would be invalid
+opt_expr
+select -assert-count 2 t:$reduce_xor
+equiv_opt -assert opt_merge
+design -load postopt
+select -assert-count 2 t:$reduce_xor
+
+# TODO pmux
+
+design -reset
+read_verilog <<EOT
+module top(A, B, X, Y);
+input [7:0] A, B;
+output X, Y;
+assign X = A > B;
+assign Y = A > B;
+endmodule
+EOT
+# Exercise the general case in hash_cell_inputs - accept
+opt_expr
+select -assert-count 2 t:$gt
+equiv_opt -assert opt_merge
+design -load postopt
+select -assert-count 1 t:$gt
+
+design -reset
+read_verilog <<EOT
+module top(A, B, C, X, Y);
+input [7:0] A, B, C;
+output X, Y;
+assign X = A > B;
+assign Y = A > C; // <- look here
+endmodule
+EOT
+# Exercise the general case in hash_cell_inputs - reject
+opt_expr
+select -assert-count 2 t:$gt
+opt_merge
+select -assert-count 2 t:$gt
From 1d773b50a4bc44cd9b431387adf053d1998a4590 Mon Sep 17 00:00:00 2001
From: "Emil J. Tywoniak" <emil@tywoniak.eu>
Date: Mon, 10 Mar 2025 13:02:49 +0100
Subject: [PATCH 7/7] opt_merge: fix dangling pointers in known_cells when keep
attribute is used
---
passes/opt/opt_merge.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/passes/opt/opt_merge.cc b/passes/opt/opt_merge.cc
index 745b27e87..b519d33d6 100644
--- a/passes/opt/opt_merge.cc
+++ b/passes/opt/opt_merge.cc
@@ -154,7 +154,7 @@ struct OptMergeWorker
bool compare_cell_parameters_and_connections(const RTLIL::Cell *cell1, const RTLIL::Cell *cell2) const
{
- log_assert(cell1 != cell2);
+ if (cell1 == cell2) return true;
if (cell1->type != cell2->type) return false;
if (cell1->parameters != cell2->parameters)
@@ -313,6 +313,8 @@ struct OptMergeWorker
if (cell->has_keep_attr()) {
if (other_cell->has_keep_attr())
continue;
+ known_cells.erase(other_cell);
+ known_cells.insert(cell);
std::swap(other_cell, cell);
}