From eb30d66d01d132f8f1fd388d4fe67aaee4e9c1c5 Mon Sep 17 00:00:00 2001 From: Alberto Gonzalez Date: Thu, 19 Mar 2020 06:15:53 +0000 Subject: [PATCH 1/3] Clean up pseudo-private member usage in `frontends/ast/ast.cc`. --- frontends/ast/ast.cc | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc index 650c7a937..57d51fbba 100644 --- a/frontends/ast/ast.cc +++ b/frontends/ast/ast.cc @@ -1281,9 +1281,9 @@ AstNode * AST::find_modport(AstNode *intf, std::string name) // Iterate over all wires in an interface and add them as wires in the AST module: void AST::explode_interface_port(AstNode *module_ast, RTLIL::Module * intfmodule, std::string intfname, AstNode *modport) { - for (auto &wire_it : intfmodule->wires_){ - AstNode *wire = new AstNode(AST_WIRE, new AstNode(AST_RANGE, AstNode::mkconst_int(wire_it.second->width -1, true), AstNode::mkconst_int(0, true))); - std::string origname = log_id(wire_it.first); + for (auto w : intfmodule->wires()){ + AstNode *wire = new AstNode(AST_WIRE, new AstNode(AST_RANGE, AstNode::mkconst_int(w->width -1, true), AstNode::mkconst_int(0, true))); + std::string origname = log_id(w->name); std::string newname = intfname + "." + origname; wire->str = newname; if (modport != NULL) { @@ -1326,9 +1326,9 @@ void AstModule::reprocess_module(RTLIL::Design *design, dictwires_){ - AstNode *wire = new AstNode(AST_WIRE, new AstNode(AST_RANGE, AstNode::mkconst_int(wire_it.second->width -1, true), AstNode::mkconst_int(0, true))); - std::string newname = log_id(wire_it.first); + for (auto w : intfmodule->wires()){ + AstNode *wire = new AstNode(AST_WIRE, new AstNode(AST_RANGE, AstNode::mkconst_int(w->width -1, true), AstNode::mkconst_int(0, true))); + std::string newname = log_id(w->name); newname = intfname + "." + newname; wire->str = newname; new_ast->children.push_back(wire); @@ -1352,7 +1352,7 @@ void AstModule::reprocess_module(RTLIL::Design *design, dict res = split_modport_from_type(ch->str); std::string interface_type = res.first; std::string interface_modport = res.second; // Is "", if no modport - if (design->modules_.count(interface_type) > 0) { + if (design->module(interface_type) != nullptr) { // Add a cell to the module corresponding to the interface port such that // it can further propagated down if needed: AstNode *celltype_for_intf = new AstNode(AST_CELLTYPE); @@ -1362,7 +1362,7 @@ void AstModule::reprocess_module(RTLIL::Design *design, dictchildren.push_back(cell_for_intf); // Get all members of this non-overridden dummy interface instance: - RTLIL::Module *intfmodule = design->modules_[interface_type]; // All interfaces should at this point in time (assuming + RTLIL::Module *intfmodule = design->module(interface_type); // All interfaces should at this point in time (assuming // reprocess_module is called from the hierarchy pass) be // present in design->modules_ AstModule *ast_module_of_interface = (AstModule*)intfmodule; @@ -1456,10 +1456,10 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dictmodule(modname); // Now that the interfaces have been exploded, we can delete the dummy port related to every interface. + pool to_remove; for(auto &intf : interfaces) { - if(mod->wires_.count(intf.first)) { - mod->wires_.erase(intf.first); - mod->fixup_ports(); + if(mod->wire(intf.first) != nullptr) { + to_remove.insert(mod->wire(intf.first)); // We copy the cell of the interface to the sub-module such that it can further be found if it is propagated // down to sub-sub-modules etc. RTLIL::Cell * new_subcell = mod->addCell(intf.first, intf.second->name); @@ -1469,6 +1469,8 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dictremove(to_remove); + mod->fixup_ports(); // If any interfaces were replaced, set the attribute 'interfaces_replaced_in_module': if (interfaces.size() > 0) { From 60405939943cd812e146b84848be8bc9307702db Mon Sep 17 00:00:00 2001 From: Alberto Gonzalez Date: Fri, 27 Mar 2020 09:46:40 +0000 Subject: [PATCH 2/3] Revert over-aggressive change to a more modest cleanup. --- frontends/ast/ast.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc index 57d51fbba..46801d691 100644 --- a/frontends/ast/ast.cc +++ b/frontends/ast/ast.cc @@ -1456,10 +1456,12 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dictmodule(modname); // Now that the interfaces have been exploded, we can delete the dummy port related to every interface. - pool to_remove; for(auto &intf : interfaces) { if(mod->wire(intf.first) != nullptr) { + pool to_remove; to_remove.insert(mod->wire(intf.first)); + mod->remove(to_remove); + mod->fixup_ports(); // We copy the cell of the interface to the sub-module such that it can further be found if it is propagated // down to sub-sub-modules etc. RTLIL::Cell * new_subcell = mod->addCell(intf.first, intf.second->name); @@ -1469,7 +1471,6 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dictremove(to_remove); mod->fixup_ports(); // If any interfaces were replaced, set the attribute 'interfaces_replaced_in_module': From b538c6fbf231422395803f612ccfb17c7947710e Mon Sep 17 00:00:00 2001 From: Alberto Gonzalez Date: Mon, 30 Mar 2020 18:08:25 +0000 Subject: [PATCH 3/3] Add explanatory comment about inefficient wire removal and remove superfluous call to `fixup_ports()`. Co-Authored-By: Eddie Hung --- frontends/ast/ast.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc index 46801d691..24d6f56d8 100644 --- a/frontends/ast/ast.cc +++ b/frontends/ast/ast.cc @@ -1458,20 +1458,24 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dictwire(intf.first) != nullptr) { + // Normally, removing wires would be batched together as it's an + // expensive operation, however, in this case doing so would mean + // that a cell with the same name cannot be created (below)... + // Since we won't expect many interfaces to exist in a module, + // we can let this slide... pool to_remove; to_remove.insert(mod->wire(intf.first)); mod->remove(to_remove); mod->fixup_ports(); - // We copy the cell of the interface to the sub-module such that it can further be found if it is propagated - // down to sub-sub-modules etc. - RTLIL::Cell * new_subcell = mod->addCell(intf.first, intf.second->name); + // We copy the cell of the interface to the sub-module such that it + // can further be found if it is propagated down to sub-sub-modules etc. + RTLIL::Cell *new_subcell = mod->addCell(intf.first, intf.second->name); new_subcell->set_bool_attribute("\\is_interface"); } else { log_error("No port with matching name found (%s) in %s. Stopping\n", log_id(intf.first), modname.c_str()); } } - mod->fixup_ports(); // If any interfaces were replaced, set the attribute 'interfaces_replaced_in_module': if (interfaces.size() > 0) {