From 6bd03b81ec8abe656ed2478f974b76639b478711 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Fri, 13 Dec 2024 18:10:10 +0100 Subject: [PATCH 1/3] macc: Stop using the B port The B port is for single-bit summands. These can just as well be represented as an additional summand on the A port (which supports summands of arbitrary width). An upcoming `$macc_v2` cell won't be special-casing single-bit summands in any way. In preparation, make the following changes: * remove the `bit_ports` field from the `Macc` helper (instead add any single-bit summands to `ports` next to other summands) * leave `B` empty on cells emitted from `Macc::to_cell` --- Makefile | 1 + kernel/consteval.h | 3 -- kernel/macc.h | 32 ++++-------------- kernel/satgen.cc | 7 ---- passes/opt/share.cc | 2 +- passes/techmap/alumacc.cc | 2 +- passes/techmap/maccmap.cc | 24 ++++++++++---- passes/tests/test_cell.cc | 13 ++++---- tests/alumacc/macc_b_port_compat.ys | 50 +++++++++++++++++++++++++++++ tests/alumacc/run-test.sh | 6 ++++ 10 files changed, 89 insertions(+), 51 deletions(-) create mode 100644 tests/alumacc/macc_b_port_compat.ys create mode 100644 tests/alumacc/run-test.sh diff --git a/Makefile b/Makefile index 1861612a336..3359937154d 100644 --- a/Makefile +++ b/Makefile @@ -881,6 +881,7 @@ endif +cd tests/blif && bash run-test.sh +cd tests/opt && bash run-test.sh +cd tests/aiger && bash run-test.sh $(ABCOPT) + +cd tests/alumacc && bash run-test.sh $(ABCOPT) +cd tests/arch && bash run-test.sh +cd tests/arch/ice40 && bash run-test.sh $(SEEDOPT) +cd tests/arch/xilinx && bash run-test.sh $(SEEDOPT) diff --git a/kernel/consteval.h b/kernel/consteval.h index 73d05f0b35d..331d8f12854 100644 --- a/kernel/consteval.h +++ b/kernel/consteval.h @@ -315,9 +315,6 @@ struct ConstEval Macc macc; macc.from_cell(cell); - if (!eval(macc.bit_ports, undef, cell)) - return false; - for (auto &port : macc.ports) { if (!eval(port.in_a, undef, cell)) return false; diff --git a/kernel/macc.h b/kernel/macc.h index 4af08cfd8ee..55940769de2 100644 --- a/kernel/macc.h +++ b/kernel/macc.h @@ -30,14 +30,11 @@ struct Macc RTLIL::SigSpec in_a, in_b; bool is_signed, do_subtract; }; - std::vector ports; - RTLIL::SigSpec bit_ports; void optimize(int width) { std::vector new_ports; - RTLIL::SigSpec new_bit_ports; RTLIL::Const off(0, width); for (auto &port : ports) @@ -48,11 +45,6 @@ struct Macc if (GetSize(port.in_a) < GetSize(port.in_b)) std::swap(port.in_a, port.in_b); - if (GetSize(port.in_a) == 1 && GetSize(port.in_b) == 0 && !port.is_signed && !port.do_subtract) { - bit_ports.append(port.in_a); - continue; - } - if (port.in_a.is_fully_const() && port.in_b.is_fully_const()) { RTLIL::Const v = port.in_a.as_const(); if (GetSize(port.in_b)) @@ -79,12 +71,6 @@ struct Macc new_ports.push_back(port); } - for (auto &bit : bit_ports) - if (bit == State::S1) - off = const_add(off, RTLIL::Const(1, width), false, false, width); - else if (bit != State::S0) - new_bit_ports.append(bit); - if (off.as_bool()) { port_t port; port.in_a = off; @@ -94,7 +80,6 @@ struct Macc } new_ports.swap(ports); - bit_ports = new_bit_ports; } void from_cell(RTLIL::Cell *cell) @@ -102,7 +87,6 @@ struct Macc RTLIL::SigSpec port_a = cell->getPort(ID::A); ports.clear(); - bit_ports = cell->getPort(ID::B); auto config_bits = cell->getParam(ID::CONFIG); int config_cursor = 0; @@ -145,6 +129,9 @@ struct Macc ports.push_back(this_port); } + for (auto bit : cell->getPort(ID::B)) + ports.push_back(port_t{{bit}, {}, false, false}); + log_assert(config_cursor == config_width); log_assert(port_a_cursor == GetSize(port_a)); } @@ -190,11 +177,11 @@ struct Macc } cell->setPort(ID::A, port_a); - cell->setPort(ID::B, bit_ports); + cell->setPort(ID::B, {}); cell->setParam(ID::CONFIG, config_bits); cell->setParam(ID::CONFIG_WIDTH, GetSize(config_bits)); cell->setParam(ID::A_WIDTH, GetSize(port_a)); - cell->setParam(ID::B_WIDTH, GetSize(bit_ports)); + cell->setParam(ID::B_WIDTH, 0); } bool eval(RTLIL::Const &result) const @@ -219,19 +206,12 @@ struct Macc result = const_add(result, summand, port.is_signed, port.is_signed, GetSize(result)); } - for (auto bit : bit_ports) { - if (bit.wire) - return false; - result = const_add(result, bit.data, false, false, GetSize(result)); - } - return true; } bool is_simple_product() { - return bit_ports.empty() && - ports.size() == 1 && + return ports.size() == 1 && !ports[0].in_b.empty() && !ports[0].do_subtract; } diff --git a/kernel/satgen.cc b/kernel/satgen.cc index ba6d664db2b..dd15b51f3bd 100644 --- a/kernel/satgen.cc +++ b/kernel/satgen.cc @@ -743,7 +743,6 @@ bool SatGen::importCell(RTLIL::Cell *cell, int timestep) if (cell->type == ID($macc)) { std::vector a = importDefSigSpec(cell->getPort(ID::A), timestep); - std::vector b = importDefSigSpec(cell->getPort(ID::B), timestep); std::vector y = importDefSigSpec(cell->getPort(ID::Y), timestep); Macc macc; @@ -785,12 +784,6 @@ bool SatGen::importCell(RTLIL::Cell *cell, int timestep) } } - for (int i = 0; i < GetSize(b); i++) { - std::vector val(GetSize(y), ez->CONST_FALSE); - val.at(0) = b.at(i); - tmp = ez->vec_add(tmp, val); - } - if (model_undef) { std::vector undef_a = importUndefSigSpec(cell->getPort(ID::A), timestep); diff --git a/passes/opt/share.cc b/passes/opt/share.cc index 1408c512aad..5d4e2d67dfe 100644 --- a/passes/opt/share.cc +++ b/passes/opt/share.cc @@ -117,7 +117,7 @@ struct ShareWorker static int bits_macc(const Macc &m, int width) { - int bits = GetSize(m.bit_ports); + int bits = 0; for (auto &p : m.ports) bits += bits_macc_port(p, width); return bits; diff --git a/passes/techmap/alumacc.cc b/passes/techmap/alumacc.cc index e4e70004c0f..cdbe0d79d69 100644 --- a/passes/techmap/alumacc.cc +++ b/passes/techmap/alumacc.cc @@ -283,7 +283,7 @@ struct AlumaccWorker for (auto &it : sig_macc) { auto n = it.second; - RTLIL::SigSpec A, B, C = n->macc.bit_ports; + RTLIL::SigSpec A, B, C; bool a_signed = false, b_signed = false; bool subtract_b = false; alunode_t *alunode; diff --git a/passes/techmap/maccmap.cc b/passes/techmap/maccmap.cc index 2235bdef9fa..911d66cfa1b 100644 --- a/passes/techmap/maccmap.cc +++ b/passes/techmap/maccmap.cc @@ -286,19 +286,23 @@ void maccmap(RTLIL::Module *module, RTLIL::Cell *cell, bool unmap) log(" %s %s * %s (%dx%d bits, %s)\n", port.do_subtract ? "sub" : "add", log_signal(port.in_a), log_signal(port.in_b), GetSize(port.in_a), GetSize(port.in_b), port.is_signed ? "signed" : "unsigned"); - if (GetSize(macc.bit_ports) != 0) - log(" add bits %s (%d bits)\n", log_signal(macc.bit_ports), GetSize(macc.bit_ports)); - if (unmap) { typedef std::pair summand_t; std::vector summands; + RTLIL::SigSpec bit_ports; + for (auto &port : macc.ports) { summand_t this_summand; if (GetSize(port.in_b)) { this_summand.first = module->addWire(NEW_ID, width); module->addMul(NEW_ID, port.in_a, port.in_b, this_summand.first, port.is_signed); + } else if (GetSize(port.in_a) == 1 && GetSize(port.in_b) == 0 && !port.is_signed && !port.do_subtract) { + // Mimic old 'bit_ports' treatment in case it's relevant for performance, + // i.e. defer single-bit summands to be the last ones + bit_ports.append(port.in_a); + continue; } else if (GetSize(port.in_a) != width) { this_summand.first = module->addWire(NEW_ID, width); module->addPos(NEW_ID, port.in_a, this_summand.first, port.is_signed); @@ -309,7 +313,7 @@ void maccmap(RTLIL::Module *module, RTLIL::Cell *cell, bool unmap) summands.push_back(this_summand); } - for (auto &bit : macc.bit_ports) + for (auto &bit : bit_ports) summands.push_back(summand_t(bit, false)); if (GetSize(summands) == 0) @@ -346,14 +350,20 @@ void maccmap(RTLIL::Module *module, RTLIL::Cell *cell, bool unmap) else { MaccmapWorker worker(module, width); + RTLIL::SigSpec bit_ports; - for (auto &port : macc.ports) - if (GetSize(port.in_b) == 0) + for (auto &port : macc.ports) { + // Mimic old 'bit_ports' treatment in case it's relevant for performance, + // i.e. defer single-bit summands to be the last ones + if (GetSize(port.in_a) == 1 && GetSize(port.in_b) == 0 && !port.is_signed && !port.do_subtract) + bit_ports.append(port.in_a); + else if (GetSize(port.in_b) == 0) worker.add(port.in_a, port.is_signed, port.do_subtract); else worker.add(port.in_a, port.in_b, port.is_signed, port.do_subtract); + } - for (auto &bit : macc.bit_ports) + for (auto bit : bit_ports) worker.add(bit, 0); module->connect(cell->getPort(ID::Y), worker.synth()); diff --git a/passes/tests/test_cell.cc b/passes/tests/test_cell.cc index b9b4d21959a..39b7ccd3ae9 100644 --- a/passes/tests/test_cell.cc +++ b/passes/tests/test_cell.cc @@ -201,18 +201,19 @@ static RTLIL::Cell* create_gold_module(RTLIL::Design *design, RTLIL::IdString ce this_port.do_subtract = xorshift32(2) == 1; macc.ports.push_back(this_port); } - - wire = module->addWire(ID::B); - wire->width = xorshift32(mulbits_a ? xorshift32(4)+1 : xorshift32(16)+1); - wire->port_input = true; - macc.bit_ports = wire; + // Macc::to_cell sets the input ports + macc.to_cell(cell); wire = module->addWire(ID::Y); wire->width = width; wire->port_output = true; cell->setPort(ID::Y, wire); - macc.to_cell(cell); + // override the B input (macc helpers always sets an empty vector) + wire = module->addWire(ID::B); + wire->width = xorshift32(mulbits_a ? xorshift32(4)+1 : xorshift32(16)+1); + wire->port_input = true; + cell->setPort(ID::B, wire); } if (cell_type == ID($lut)) diff --git a/tests/alumacc/macc_b_port_compat.ys b/tests/alumacc/macc_b_port_compat.ys new file mode 100644 index 00000000000..1ed2ad34c1b --- /dev/null +++ b/tests/alumacc/macc_b_port_compat.ys @@ -0,0 +1,50 @@ +# We don't set the B port on $macc cells anymore, +# test compatibility with older RTLIL files which can +# have the B port populated + +read_verilog < Date: Fri, 13 Dec 2024 18:59:35 +0100 Subject: [PATCH 2/3] rtlil: Add const append helper --- kernel/rtlil.cc | 6 ++++++ kernel/rtlil.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index b279cce6e15..63258079591 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -540,6 +540,12 @@ void RTLIL::Const::bitvectorize() const { } } +void RTLIL::Const::append(const RTLIL::Const &other) { + bitvectorize(); + bitvectype& bv = get_bits(); + bv.insert(bv.end(), other.begin(), other.end()); +} + RTLIL::State RTLIL::Const::const_iterator::operator*() const { if (auto bv = parent.get_if_bits()) return (*bv)[idx]; diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 6d3558621c1..01ff94f7624 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -710,6 +710,8 @@ struct RTLIL::Const bool empty() const; void bitvectorize() const; + void append(const RTLIL::Const &other); + class const_iterator { private: const Const& parent; From abdeac3069c1626c307ed05e5ba6ae741073d778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Povi=C5=A1er?= Date: Fri, 13 Dec 2024 19:01:41 +0100 Subject: [PATCH 3/3] macc_v2: Start new cell --- kernel/constids.inc | 4 ++ kernel/macc.h | 104 ++++++++++++++++++---------- kernel/rtlil.cc | 29 ++++++++ passes/techmap/maccmap.cc | 2 +- tests/alumacc/macc_infer_n_unmap.ys | 19 +++++ 5 files changed, 120 insertions(+), 38 deletions(-) create mode 100644 tests/alumacc/macc_infer_n_unmap.ys diff --git a/kernel/constids.inc b/kernel/constids.inc index d68e2dfe646..15d3cc83b29 100644 --- a/kernel/constids.inc +++ b/kernel/constids.inc @@ -276,3 +276,7 @@ X(Y) X(Y_WIDTH) X(area) X(capacitance) +X(NTERMS) +X(TERM_NEGATED) +X(A_WIDTHS) +X(B_WIDTHS) diff --git a/kernel/macc.h b/kernel/macc.h index 55940769de2..e969dd86d0a 100644 --- a/kernel/macc.h +++ b/kernel/macc.h @@ -82,7 +82,7 @@ struct Macc new_ports.swap(ports); } - void from_cell(RTLIL::Cell *cell) + void from_cell_v1(RTLIL::Cell *cell) { RTLIL::SigSpec port_a = cell->getPort(ID::A); @@ -136,52 +136,82 @@ struct Macc log_assert(port_a_cursor == GetSize(port_a)); } - void to_cell(RTLIL::Cell *cell) const + void from_cell(RTLIL::Cell *cell) { - RTLIL::SigSpec port_a; - std::vector config_bits; - int max_size = 0, num_bits = 0; - - for (auto &port : ports) { - max_size = max(max_size, GetSize(port.in_a)); - max_size = max(max_size, GetSize(port.in_b)); + if (cell->type == ID($macc)) { + from_cell_v1(cell); + return; } + log_assert(cell->type == ID($macc_v2)); - while (max_size) - num_bits++, max_size /= 2; + RTLIL::SigSpec port_a = cell->getPort(ID::A); + RTLIL::SigSpec port_b = cell->getPort(ID::B); - log_assert(num_bits < 16); - config_bits.push_back(num_bits & 1 ? State::S1 : State::S0); - config_bits.push_back(num_bits & 2 ? State::S1 : State::S0); - config_bits.push_back(num_bits & 4 ? State::S1 : State::S0); - config_bits.push_back(num_bits & 8 ? State::S1 : State::S0); + ports.clear(); - for (auto &port : ports) - { - if (GetSize(port.in_a) == 0) - continue; + int nterms = cell->getParam(ID::NTERMS).as_int(); + const Const &neg = cell->getParam(ID::TERM_NEGATED); + const Const &a_widths = cell->getParam(ID::A_WIDTHS); + const Const &b_widths = cell->getParam(ID::B_WIDTHS); + const Const &a_signed = cell->getParam(ID::A_SIGNED); + const Const &b_signed = cell->getParam(ID::B_SIGNED); + + int ai = 0, bi = 0; + for (int i = 0; i < nterms; i++) { + port_t term; + + log_assert(a_signed[i] == b_signed[i]); + term.is_signed = (a_signed[i] == State::S1); + int a_width = a_widths.extract(16 * i, 16).as_int(false); + int b_width = b_widths.extract(16 * i, 16).as_int(false); + + term.in_a = port_a.extract(ai, a_width); + ai += a_width; + term.in_b = port_b.extract(bi, b_width); + bi += b_width; + term.do_subtract = (neg[i] == State::S1); + + ports.push_back(term); + } + log_assert(port_a.size() == ai); + log_assert(port_b.size() == bi); + } - config_bits.push_back(port.is_signed ? State::S1 : State::S0); - config_bits.push_back(port.do_subtract ? State::S1 : State::S0); + void to_cell(RTLIL::Cell *cell) + { + cell->type = ID($macc_v2); - int size_a = GetSize(port.in_a); - for (int i = 0; i < num_bits; i++) - config_bits.push_back(size_a & (1 << i) ? State::S1 : State::S0); + int nterms = ports.size(); + const auto Sx = State::Sx; + Const a_signed(Sx, nterms), b_signed(Sx, nterms), negated(Sx, nterms); + Const a_widths, b_widths; + SigSpec a, b; - int size_b = GetSize(port.in_b); - for (int i = 0; i < num_bits; i++) - config_bits.push_back(size_b & (1 << i) ? State::S1 : State::S0); + for (int i = 0; i < nterms; i++) { + SigSpec term_a = ports[i].in_a, term_b = ports[i].in_b; - port_a.append(port.in_a); - port_a.append(port.in_b); - } + a_widths.append(Const(term_a.size(), 16)); + b_widths.append(Const(term_b.size(), 16)); - cell->setPort(ID::A, port_a); - cell->setPort(ID::B, {}); - cell->setParam(ID::CONFIG, config_bits); - cell->setParam(ID::CONFIG_WIDTH, GetSize(config_bits)); - cell->setParam(ID::A_WIDTH, GetSize(port_a)); - cell->setParam(ID::B_WIDTH, 0); + a_signed.bits()[i] = b_signed.bits()[i] = + (ports[i].is_signed ? RTLIL::S1 : RTLIL::S0); + negated.bits()[i] = (ports[i].do_subtract ? RTLIL::S1 : RTLIL::S0); + + a.append(term_a); + b.append(term_b); + } + negated.is_fully_def(); + a_signed.is_fully_def(); + b_signed.is_fully_def(); + + cell->setParam(ID::NTERMS, nterms); + cell->setParam(ID::TERM_NEGATED, negated); + cell->setParam(ID::A_SIGNED, a_signed); + cell->setParam(ID::B_SIGNED, b_signed); + cell->setParam(ID::A_WIDTHS, a_widths); + cell->setParam(ID::B_WIDTHS, b_widths); + cell->setPort(ID::A, a); + cell->setPort(ID::B, b); } bool eval(RTLIL::Const &result) const diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 63258079591..ca6c24a6fa8 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -1467,6 +1467,30 @@ namespace { return; } + if (cell->type == ID($macc_v2)) { + if (param(ID::NTERMS) <= 0) + error(__LINE__); + param_bits(ID::TERM_NEGATED, param(ID::NTERMS)); + param_bits(ID::A_SIGNED, param(ID::NTERMS)); + param_bits(ID::B_SIGNED, param(ID::NTERMS)); + if (cell->getParam(ID::A_SIGNED) != cell->getParam(ID::B_SIGNED)) + error(__LINE__); + param_bits(ID::A_WIDTHS, param(ID::NTERMS) * 16); + param_bits(ID::B_WIDTHS, param(ID::NTERMS) * 16); + const Const &a_width = cell->getParam(ID::A_WIDTHS); + const Const &b_width = cell->getParam(ID::B_WIDTHS); + int a_width_sum = 0, b_width_sum = 0; + for (int i = 0; i < param(ID::NTERMS); i++) { + a_width_sum += a_width.extract(16 * i, 16).as_int(false); + b_width_sum += b_width.extract(16 * i, 16).as_int(false); + } + port(ID::A, a_width_sum); + port(ID::B, b_width_sum); + port(ID::Y, param(ID::Y_WIDTH)); + check_expected(); + return; + } + if (cell->type == ID($logic_not)) { param_bool(ID::A_SIGNED); port(ID::A, param(ID::A_WIDTH)); @@ -4099,6 +4123,11 @@ void RTLIL::Cell::fixup_parameters(bool set_a_signed, bool set_b_signed) return; } + if (type == ID($macc_v2)) { + parameters[ID::Y_WIDTH] = GetSize(connections_[ID::Y]); + return; + } + bool signedness_ab = !type.in(ID($slice), ID($concat), ID($macc)); if (connections_.count(ID::A)) { diff --git a/passes/techmap/maccmap.cc b/passes/techmap/maccmap.cc index 911d66cfa1b..3dde9243803 100644 --- a/passes/techmap/maccmap.cc +++ b/passes/techmap/maccmap.cc @@ -403,7 +403,7 @@ struct MaccmapPass : public Pass { for (auto mod : design->selected_modules()) for (auto cell : mod->selected_cells()) - if (cell->type == ID($macc)) { + if (cell->type.in(ID($macc), ID($macc_v2))) { log("Mapping %s.%s (%s).\n", log_id(mod), log_id(cell), log_id(cell->type)); maccmap(mod, cell, unmap_mode); mod->remove(cell); diff --git a/tests/alumacc/macc_infer_n_unmap.ys b/tests/alumacc/macc_infer_n_unmap.ys new file mode 100644 index 00000000000..569511b64d0 --- /dev/null +++ b/tests/alumacc/macc_infer_n_unmap.ys @@ -0,0 +1,19 @@ +read_verilog <