From a56d6970f24ae6044e02bea333c484035fd5cdfa Mon Sep 17 00:00:00 2001 From: Eddie Hung Date: Fri, 27 Dec 2019 16:05:58 -0800 Subject: [PATCH 1/3] Revert "Merge pull request #1598 from YosysHQ/revert-1588-eddie/xaiger_cleanup" This reverts commit 92654f73ea92ee9e390c8ab50d8cb51c47a7ffa9, reversing changes made to 3e14ff16676884a1f65cf0eeb0ca9cb1958b8804. --- backends/aiger/xaiger.cc | 46 +++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/backends/aiger/xaiger.cc b/backends/aiger/xaiger.cc index 3599e19e3..68d1b1e69 100644 --- a/backends/aiger/xaiger.cc +++ b/backends/aiger/xaiger.cc @@ -599,15 +599,25 @@ struct XAigerWriter RTLIL::Module *holes_module = module->design->addModule("$__holes__"); log_assert(holes_module); + dict cell_cache; + int port_id = 1; int box_count = 0; for (auto cell : box_list) { RTLIL::Module* box_module = module->design->module(cell->type); + log_assert(box_module); + IdString derived_name = box_module->derive(module->design, cell->parameters); + box_module = module->design->module(derived_name); + if (box_module->has_processes()) + log_error("ABC9 box '%s' contains processes!\n", box_module->name.c_str()); + int box_inputs = 0, box_outputs = 0; - Cell *holes_cell = nullptr; - if (box_module->get_bool_attribute("\\whitebox")) { + auto r = cell_cache.insert(std::make_pair(derived_name, nullptr)); + Cell *holes_cell = r.first->second; + if (r.second && !holes_cell && box_module->get_bool_attribute("\\whitebox")) { holes_cell = holes_module->addCell(cell->name, cell->type); holes_cell->parameters = cell->parameters; + r.first->second = holes_cell; } // NB: Assume box_module->ports are sorted alphabetically @@ -616,8 +626,8 @@ struct XAigerWriter RTLIL::Wire *w = box_module->wire(port_name); log_assert(w); RTLIL::Wire *holes_wire; - RTLIL::SigSpec port_wire; - if (w->port_input) { + RTLIL::SigSpec port_sig; + if (w->port_input) for (int i = 0; i < GetSize(w); i++) { box_inputs++; holes_wire = holes_module->wire(stringf("\\i%d", box_inputs)); @@ -628,28 +638,29 @@ struct XAigerWriter holes_module->ports.push_back(holes_wire->name); } if (holes_cell) - port_wire.append(holes_wire); + port_sig.append(holes_wire); } - if (!port_wire.empty()) - holes_cell->setPort(w->name, port_wire); - } if (w->port_output) { box_outputs += GetSize(w); for (int i = 0; i < GetSize(w); i++) { if (GetSize(w) == 1) - holes_wire = holes_module->addWire(stringf("%s.%s", cell->name.c_str(), w->name.c_str())); + holes_wire = holes_module->addWire(stringf("%s.%s", cell->name.c_str(), log_id(w->name))); else - holes_wire = holes_module->addWire(stringf("%s.%s[%d]", cell->name.c_str(), w->name.c_str(), i)); + holes_wire = holes_module->addWire(stringf("%s.%s[%d]", cell->name.c_str(), log_id(w->name), i)); holes_wire->port_output = true; holes_wire->port_id = port_id++; holes_module->ports.push_back(holes_wire->name); if (holes_cell) - port_wire.append(holes_wire); + port_sig.append(holes_wire); else holes_module->connect(holes_wire, State::S0); } - if (!port_wire.empty()) - holes_cell->setPort(w->name, port_wire); + } + if (!port_sig.empty()) { + if (r.second) + holes_cell->setPort(w->name, port_sig); + else + holes_module->connect(holes_cell->getPort(w->name), port_sig); } } @@ -679,14 +690,11 @@ struct XAigerWriter RTLIL::Selection& sel = holes_module->design->selection_stack.back(); sel.select(holes_module); - // TODO: Should not need to opt_merge if we only instantiate - // each box type once... - Pass::call(holes_module->design, "opt_merge -share_all"); - Pass::call(holes_module->design, "flatten -wb"); - // TODO: Should techmap/aigmap/check all lib_whitebox-es just once, - // instead of per write_xaiger call + // Cannot techmap/aigmap/check all lib_whitebox-es outside of write_xaiger + // since boxes may contain parameters in which case `flatten` would have + // created a new $paramod ... Pass::call(holes_module->design, "techmap"); Pass::call(holes_module->design, "aigmap"); for (auto cell : holes_module->cells()) From 237415e78cab2c15d783657c4c2bc959efb298bb Mon Sep 17 00:00:00 2001 From: Eddie Hung Date: Fri, 27 Dec 2019 16:44:18 -0800 Subject: [PATCH 2/3] write_xaiger: inherit port ordering from original module --- backends/aiger/xaiger.cc | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/backends/aiger/xaiger.cc b/backends/aiger/xaiger.cc index 68d1b1e69..445103771 100644 --- a/backends/aiger/xaiger.cc +++ b/backends/aiger/xaiger.cc @@ -604,27 +604,38 @@ struct XAigerWriter int port_id = 1; int box_count = 0; for (auto cell : box_list) { - RTLIL::Module* box_module = module->design->module(cell->type); - log_assert(box_module); - IdString derived_name = box_module->derive(module->design, cell->parameters); - box_module = module->design->module(derived_name); + RTLIL::Module* orig_box_module = module->design->module(cell->type); + log_assert(orig_box_module); + IdString derived_name = orig_box_module->derive(module->design, cell->parameters); + RTLIL::Module* box_module = module->design->module(derived_name); if (box_module->has_processes()) log_error("ABC9 box '%s' contains processes!\n", box_module->name.c_str()); int box_inputs = 0, box_outputs = 0; auto r = cell_cache.insert(std::make_pair(derived_name, nullptr)); Cell *holes_cell = r.first->second; - if (r.second && !holes_cell && box_module->get_bool_attribute("\\whitebox")) { + if (r.second && box_module->get_bool_attribute("\\whitebox")) { holes_cell = holes_module->addCell(cell->name, cell->type); holes_cell->parameters = cell->parameters; r.first->second = holes_cell; + + // Since Module::derive() will create a new module, there + // is a chance that the ports will be alphabetically ordered + // again, which is a problem when carry-chains are involved. + // Inherit the port ordering from the original module here... + // (and set the port_id below, when iterating through those) + log_assert(GetSize(box_module->ports) == GetSize(orig_box_module->ports)); + box_module->ports = orig_box_module->ports; } // NB: Assume box_module->ports are sorted alphabetically // (as RTLIL::Module::fixup_ports() would do) + int box_port_id = 1; for (const auto &port_name : box_module->ports) { RTLIL::Wire *w = box_module->wire(port_name); log_assert(w); + if (r.second) + w->port_id = box_port_id++; RTLIL::Wire *holes_wire; RTLIL::SigSpec port_sig; if (w->port_input) From d45869855c6fc86dc6a0225018a8e383866dacb4 Mon Sep 17 00:00:00 2001 From: Eddie Hung Date: Fri, 27 Dec 2019 16:44:57 -0800 Subject: [PATCH 3/3] Add #1598 testcase --- tests/arch/ecp5/bug1598.ys | 16 ++++++++++++++++ tests/arch/ice40/bug1598.ys | 16 ++++++++++++++++ tests/arch/xilinx/bug1598.ys | 16 ++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 tests/arch/ecp5/bug1598.ys create mode 100644 tests/arch/ice40/bug1598.ys create mode 100644 tests/arch/xilinx/bug1598.ys diff --git a/tests/arch/ecp5/bug1598.ys b/tests/arch/ecp5/bug1598.ys new file mode 100644 index 000000000..1d1682fcd --- /dev/null +++ b/tests/arch/ecp5/bug1598.ys @@ -0,0 +1,16 @@ +read_verilog <