Simplify a modport check in hierarchy.cc
authorRupert Swarbrick <rswarbrick@gmail.com>
Tue, 26 May 2020 15:21:38 +0000 (16:21 +0100)
committerRupert Swarbrick <rswarbrick@gmail.com>
Tue, 26 May 2020 15:50:42 +0000 (16:50 +0100)
This code originally comes from commit 458a940. When an interface is
used via a modport, code in genrtlil.cc sets '\\interface_type' and
'\\interface_modport' properties on the wire.

In hierarchy.cc, we pick up the modport name and add it to a dict
called modports_used_in_submodule (that maps connection source to
modport name).

Before this patch, the modport name is retrieved as a strpool and then
iterated over in an arbitrary order, discarding all entries but the
last. In practice, the pool will always have 0 or 1 entries because
the string used to construct it is a valid identifier, so doesn't
contain any pipe symbols.

This patch changes the code to retrieve the modport name as just a
string. This will have the same effect in practice, but may be a bit
less confusing!

The code also gets moved down closer to where the result is used,
which might be a bit more efficient since we won't always get as far
as the check.

The patch also removes some commented-out code, which I think was
intended to add some typechecking at some point, but was never
implemented. Since this dates back to October 2018, I think it makes
more sense to just take it out.

passes/hierarchy/hierarchy.cc

index 95d74d1eb2b9dd25f4d7829d4d9f9d0bd4863070..12091c9f0796cb5036957acfd1ec7221a98a31a6 100644 (file)
@@ -254,16 +254,6 @@ bool expand_module(RTLIL::Design *design, RTLIL::Module *module, bool flag_check
                // some lists, so that the ports for sub-modules can be replaced further down:
                for (auto &conn : cell->connections()) {
                        if(mod->wire(conn.first) != nullptr && mod->wire(conn.first)->get_bool_attribute(ID::is_interface)) { // Check if the connection is present as an interface in the sub-module's port list
-                               //const pool<string> &interface_type_pool = mod->wire(conn.first)->get_strpool_attribute(ID::interface_type);
-                               //for (auto &d : interface_type_pool) { // TODO: Compare interface type to type in parent module (not crucially important, but good for robustness)
-                               //}
-
-                               // Find if the sub-module has set a modport for the current interface connection:
-                               const pool<string> &interface_modport_pool = mod->wire(conn.first)->get_strpool_attribute(ID::interface_modport);
-                               std::string interface_modport = "";
-                               for (auto &d : interface_modport_pool) {
-                                       interface_modport = "\\" + d;
-                               }
                                if(conn.second.bits().size() == 1 && conn.second.bits()[0].wire->get_bool_attribute(ID::is_interface)) { // Check if the connected wire is a potential interface in the parent module
                                        std::string interface_name_str = conn.second.bits()[0].wire->name.str();
                                        interface_name_str.replace(0,23,""); // Strip the prefix '$dummywireforinterface' from the dummy wire to get the name
@@ -297,9 +287,12 @@ bool expand_module(RTLIL::Design *design, RTLIL::Module *module, bool flag_check
                                                        connections_to_remove.push_back(conn.first);
                                                        interfaces_to_add_to_submodule[conn.first] = interfaces_in_module.at(interface_name2);
 
-                                                       // Add modports to a dict which will be passed to AstModule::derive
-                                                       if (interface_modport != "") {
-                                                               modports_used_in_submodule[conn.first] = interface_modport;
+                                                       // Find if the sub-module has set a modport for the current
+                                                       // interface connection. Add any modports to a dict which will
+                                                       // be passed to AstModule::derive
+                                                       string modport_name = mod->wire(conn.first)->get_string_attribute(ID::interface_modport);
+                                                       if (!modport_name.empty()) {
+                                                               modports_used_in_submodule[conn.first] = "\\" + modport_name;
                                                        }
                                                }
                                                else not_found_interface = true;