Documentation improvements etc.
authorRuben Undheim <ruben.undheim@gmail.com>
Sat, 13 Oct 2018 18:34:44 +0000 (20:34 +0200)
committerRuben Undheim <ruben.undheim@gmail.com>
Sat, 13 Oct 2018 18:34:44 +0000 (20:34 +0200)
- Mention new feature in the SystemVerilog section in the README file
- Commented changes much better
- Rename a few signals to make it clearer
- Prevent warning for unused signals in an easier way
- Add myself as copyright holder to 2 files
- Fix one potential memory leak (delete 'wire' if not in modport)

README.md
frontends/ast/ast.cc
frontends/ast/genrtlil.cc
kernel/rtlil.cc
passes/hierarchy/hierarchy.cc

index 41ae4ac18d9cc353681677025733abe8e49af0c4..8c5dd94191a07ee9c1b66088bc2e2a257ff05464 100644 (file)
--- a/README.md
+++ b/README.md
@@ -452,6 +452,9 @@ from SystemVerilog:
   into a design with ``read_verilog``, all its packages are available to
   SystemVerilog files being read into the same design afterwards.
 
+- SystemVerilog interfaces (SVIs) are supported. Modports for specifying whether
+  ports are inputs or outputs are supported.
+
 
 Building the documentation
 ==========================
index b3f78c9225246006db46d6b78e40c16640367722..7600e2912c8f6a800e06f27782c530d9745294ff 100644 (file)
@@ -2,6 +2,7 @@
  *  yosys -- Yosys Open SYnthesis Suite
  *
  *  Copyright (C) 2012  Clifford Wolf <clifford@clifford.at>
+ *  Copyright (C) 2018  Ruben Undheim <ruben.undheim@gmail.com>
  *
  *  Permission to use, copy, modify, and/or distribute this software for any
  *  purpose with or without fee is hereby granted, provided that the above
@@ -1086,6 +1087,8 @@ AstModule::~AstModule()
                delete ast;
 }
 
+// When an interface instance is found in a module, the whole RTLIL for the module will be rederived again
+// from AST. The interface members are copied into the AST module with the prefix of the interface.
 void AstModule::reprocess_module(RTLIL::Design *design, dict<RTLIL::IdString, RTLIL::Module*> local_interfaces)
 {
        bool is_top = false;
@@ -1101,23 +1104,33 @@ void AstModule::reprocess_module(RTLIL::Design *design, dict<RTLIL::IdString, RT
                        new_ast->children.push_back(wire);
                }
        }
+
+       // The old module will be deleted. Rename and mark for deletion:
        std::string original_name = this->name.str();
        std::string changed_name = original_name + "_before_replacing_local_interfaces";
        design->rename(this, changed_name);
        this->set_bool_attribute("\\to_delete");
+
+       // Check if the module was the top module. If it was, we need to remove the top attribute and put it on the
+       // new module.
        if (this->get_bool_attribute("\\initial_top")) {
                this->attributes.erase("\\initial_top");
                is_top = true;
        }
+
+       // Generate RTLIL from AST for the new module and add to the design:
        AstModule *newmod = process_module(new_ast, false);
        design->add(newmod);
        RTLIL::Module* mod = design->module(original_name);
        if (is_top)
                mod->set_bool_attribute("\\top");
+
+       // Set the attribute "interfaces_replaced_in_module" so that it does not happen again.
        mod->set_bool_attribute("\\interfaces_replaced_in_module");
 }
 
 // create a new parametric module (when needed) and return the name of the generated module - WITH support for interfaces
+// This method is used to explode the interface when the interface is a port of the module (not instantiated inside)
 RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, RTLIL::Const> parameters, dict<RTLIL::IdString, RTLIL::Module*> interfaces, dict<RTLIL::IdString, RTLIL::IdString> modports, bool mayfail)
 {
        AstNode *new_ast = NULL;
@@ -1140,9 +1153,12 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, R
 
        if (!design->has(modname)) {
                new_ast->str = modname;
+
+               // Iterate over all interfaces which are ports in this module:
                for(auto &intf : interfaces) {
                        RTLIL::Module * intfmodule = intf.second;
                        std::string intfname = intf.first.str();
+                       // Check if a modport applies for the interface port:
                        AstNode *modport = NULL;
                        if (modports.count(intfname) > 0) {
                                std::string interface_modport = modports.at(intfname).str();
@@ -1150,12 +1166,13 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, R
                                AstNode *ast_node_of_interface = ast_module_of_interface->ast;
                                for (auto &ch : ast_node_of_interface->children) {
                                        if (ch->type == AST_MODPORT) {
-                                               if (ch->str == interface_modport) {
+                                               if (ch->str == interface_modport) { // Modport found
                                                        modport = ch;
                                                }
                                        }
                                }
                        }
+                       // Iterate over all wires in the interface and add them to the module:
                        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);
@@ -1163,10 +1180,11 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, R
                                wire->str = newname;
                                if (modport != NULL) {
                                        bool found_in_modport = false;
+                                       // Search for the current wire in the wire list for the current modport
                                        for (auto &ch : modport->children) {
                                                if (ch->type == AST_MODPORTMEMBER) {
                                                        std::string compare_name = "\\" + origname;
-                                                       if (ch->str == compare_name) {
+                                                       if (ch->str == compare_name) { // Found signal. The modport decides whether it is input or output
                                                                found_in_modport = true;
                                                                wire->is_input = ch->is_input;
                                                                wire->is_output = ch->is_output;
@@ -1174,9 +1192,12 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, R
                                                        }
                                                }
                                        }
-                                       if (found_in_modport) { // If not found in modport, do not create port
+                                       if (found_in_modport) {
                                                new_ast->children.push_back(wire);
                                        }
+                                       else { // If not found in modport, do not create port
+                                               delete wire;
+                                       }
                                }
                                else { // If no modport, set inout
                                        wire->is_input = true;
@@ -1191,10 +1212,13 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, R
 
                RTLIL::Module* mod = design->module(modname);
 
+               // Now that the interfaces have been exploded, we can delete the dummy port related to every interface.
                for(auto &intf : interfaces) {
                        if(mod->wires_.count(intf.first)) {
                                mod->wires_.erase(intf.first);
                                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);
                                new_subcell->set_bool_attribute("\\is_interface");
                        }
@@ -1203,6 +1227,7 @@ RTLIL::IdString AstModule::derive(RTLIL::Design *design, dict<RTLIL::IdString, R
                        }
                }
 
+               // If any interfaces were replaced, set the attribute 'interfaces_replaced_in_module':
                if (interfaces.size() > 0) {
                        mod->set_bool_attribute("\\interfaces_replaced_in_module");
                }
index d87163dc2e48d48f98bbd1fa9a454d7113a93822..32b9af6e9a8afd327cf7b094114cb0c56f604b5a 100644 (file)
@@ -857,7 +857,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
        case AST_MODPORTMEMBER:
                break;
        case AST_INTERFACEPORT: {
-               // If a port in a module with unknown type is found, mark it as "is_interface=true"
+               // If a port in a module with unknown type is found, mark it with the attribute 'is_interface'
                // This is used by the hierarchy pass to know when it can replace interface connection with the individual
                // signals.
                RTLIL::Wire *wire = current_module->addWire(str, 1);
@@ -872,7 +872,8 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                                if(children[i]->type == AST_INTERFACEPORTTYPE) {
                                        std::string name_type = children[i]->str;
                                        size_t ndots = std::count(name_type.begin(), name_type.end(), '.');
-                                       if (ndots == 0) {
+                                       // Separate the interface instance name from any modports:
+                                       if (ndots == 0) { // Does not have modport
                                                wire->attributes["\\interface_type"] = name_type;
                                        }
                                        else {
@@ -882,11 +883,11 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                                                while(std::getline(name_type_stream, segment, '.')) {
                                                        seglist.push_back(segment);
                                                }
-                                               if (ndots == 1) {
+                                               if (ndots == 1) { // Has modport
                                                        wire->attributes["\\interface_type"] = seglist[0];
                                                        wire->attributes["\\interface_modport"] = seglist[1];
                                                }
-                                               else {
+                                               else { // Erroneous port type
                                                        log_error("More than two '.' in signal port type (%s)\n", name_type.c_str());
                                                }
                                        }
@@ -1034,7 +1035,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                                log_file_error(filename, linenum, "Identifier `%s' does map to an unexpanded memory!\n",
                                               str.c_str());
 
-                       // If identifier is an interface, create a RTLIL::SigSpec object and set is_interface to true.
+                       // If identifier is an interface, create a RTLIL::SigSpec with a dummy wire with a attribute called 'is_interface'
                        // This makes it possible for the hierarchy pass to see what are interface connections and then replace them
                        // with the individual signals:
                        if (is_interface) {
@@ -1495,6 +1496,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 
                        RTLIL::Cell *cell = current_module->addCell(str, "");
                        cell->attributes["\\src"] = stringf("%s:%d", filename.c_str(), linenum);
+                       // Set attribute 'module_not_derived' which will be cleared again after the hierarchy pass
                        cell->set_bool_attribute("\\module_not_derived");
 
                        for (auto it = children.begin(); it != children.end(); it++) {
index 07dd4bfa0145186b49dd856c476858b75b0d2ed7..14259f8ed3c97c6f329a37815b3eba6997e6e53d 100644 (file)
@@ -639,11 +639,9 @@ RTLIL::Module::~Module()
                delete it->second;
 }
 
-void RTLIL::Module::reprocess_module(RTLIL::Design *design, dict<RTLIL::IdString, RTLIL::Module *> local_interfaces)
+void RTLIL::Module::reprocess_module(RTLIL::Design *, dict<RTLIL::IdString, RTLIL::Module *>)
 {
        log_error("Cannot reprocess_module module `%s' !\n", id2cstr(name));
-       (void)local_interfaces; // To remove build warning
-       (void)design;           // To remove build warning
 }
 
 RTLIL::IdString RTLIL::Module::derive(RTLIL::Design*, dict<RTLIL::IdString, RTLIL::Const>, bool mayfail)
index f2f0d6e5b210798c8beb2eca3498cb076b78d92b..7e93a6f9af67671f1c875470801a3791a4e397e2 100644 (file)
@@ -2,6 +2,7 @@
  *  yosys -- Yosys Open SYnthesis Suite
  *
  *  Copyright (C) 2012  Clifford Wolf <clifford@clifford.at>
+ *  Copyright (C) 2018  Ruben Undheim <ruben.undheim@gmail.com>
  *
  *  Permission to use, copy, modify, and/or distribute this software for any
  *  purpose with or without fee is hereby granted, provided that the above
@@ -145,6 +146,7 @@ bool expand_module(RTLIL::Design *design, RTLIL::Module *module, bool flag_check
        std::map<RTLIL::Cell*, std::pair<int, int>> array_cells;
        std::string filename;
 
+       // Always keep track of all derived interfaces available in the current module in 'interfaces_in_module':
        dict<RTLIL::IdString, RTLIL::Module*> interfaces_in_module;
        for (auto &cell_it : module->cells_)
        {
@@ -222,50 +224,54 @@ bool expand_module(RTLIL::Design *design, RTLIL::Module *module, bool flag_check
                RTLIL::Module *mod = design->module(cell->type);
 
                // Go over all connections and see if any of them are SV interfaces. If they are, then add the replacements to
-               // some lists, so that they can be replaced further down:
+               // some lists, so that the ports for sub-modules can be replaced further down:
                for (auto &conn : cell->connections()) {
                        if(mod->wires_.count(conn.first) != 0 && mod->wire(conn.first)->get_bool_attribute("\\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("\\interface_type");
-                               //for (auto &d : interface_type_pool) { // TODO: Compare interface type to type in parent module
+                               //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("\\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("\\is_interface")) {
+                               if(conn.second.bits().size() == 1 && conn.second.bits()[0].wire->get_bool_attribute("\\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,"");
+                                       interface_name_str.replace(0,23,""); // Strip the prefix '$dummywireforinterface' from the dummy wire to get the name
                                        interface_name_str = "\\" + interface_name_str;
                                        RTLIL::IdString interface_name = interface_name_str;
-                                       bool will_do_step = false;
-                                       if(module->get_bool_attribute("\\interfaces_replaced_in_module")) {
+                                       bool not_found_interface = false;
+                                       if(module->get_bool_attribute("\\interfaces_replaced_in_module")) { // If 'interfaces' in the cell have not be been handled yet, there is no need to derive the sub-module either
                                                if (interfaces_in_module.count(interface_name) > 0) { // Check if the interface instance is present in module
                                                        RTLIL::Module *mod_replace_ports = interfaces_in_module.at(interface_name);
-                                               for (auto &mod_wire : mod_replace_ports->wires_) {
-                                                       std::string signal_name1 = conn.first.str() + "." + log_id(mod_wire.first);
-                                                       std::string signal_name2 = interface_name.str() + "." + log_id(mod_wire.first);
-                                                       connections_to_add_name.push_back(RTLIL::IdString(signal_name1));
-                                                       if(module->wires_.count(signal_name2) == 0) {
-                                                               log_error("Could not find signal '%s' in '%s'\n", signal_name2.c_str(), log_id(module->name));
+                                                       for (auto &mod_wire : mod_replace_ports->wires_) { // Go over all wires in interface, and add replacements to lists.
+                                                               std::string signal_name1 = conn.first.str() + "." + log_id(mod_wire.first);
+                                                               std::string signal_name2 = interface_name.str() + "." + log_id(mod_wire.first);
+                                                               connections_to_add_name.push_back(RTLIL::IdString(signal_name1));
+                                                               if(module->wires_.count(signal_name2) == 0) {
+                                                                       log_error("Could not find signal '%s' in '%s'\n", signal_name2.c_str(), log_id(module->name));
+                                                               }
+                                                               else {
+                                                                       RTLIL::Wire *wire_in_parent = module->wire(signal_name2);
+                                                                       connections_to_add_signal.push_back(wire_in_parent);
+                                                               }
                                                        }
-                                                       else {
-                                                               RTLIL::Wire *wire_in_parent = module->wire(signal_name2);
-                                                               connections_to_add_signal.push_back(wire_in_parent);
+                                                       connections_to_remove.push_back(conn.first);
+                                                       interfaces_to_add_to_submodule[conn.first] = interfaces_in_module.at(interface_name);
+
+                                                       // Add modports to a dict which will be passed to AstModule::derive
+                                                       if (interface_modport != "") {
+                                                               modports_used_in_submodule[conn.first] = interface_modport;
                                                        }
                                                }
-                                               connections_to_remove.push_back(conn.first);
-                                               interfaces_to_add_to_submodule[conn.first] = interfaces_in_module.at(interface_name);
-                                               if (interface_modport != "") {
-                                                       modports_used_in_submodule[conn.first] = interface_modport;
-                                               }
-                                         }
-                                         else will_do_step = true;
+                                               else not_found_interface = true;
                                        }
-                                       else will_do_step = true;
+                                       else not_found_interface = true;
                                        // If the interface instance has not already been derived in the module, we cannot complete at this stage. Set "has_interfaces_not_found"
                                        // which will delay the expansion of this cell:
-                                       if (will_do_step) {
+                                       if (not_found_interface) {
                                                // If we have already gone over all cells in this module, and the interface has still not been found - flag it as an error:
                                                if(!(module->get_bool_attribute("\\cells_not_processed"))) {
                                                        log_warning("Could not find interface instance for `%s' in `%s'\n", log_id(interface_name), log_id(module));
@@ -348,13 +354,16 @@ bool expand_module(RTLIL::Design *design, RTLIL::Module *module, bool flag_check
                                interfaces_in_module[cell->name] = derived_module;
                                did_something = true;
                        }
-               // We unset 'module_not_derived' such that we will not rederive the cell again (needed when there are interfaces connected to the cell)
+               // We clear 'module_not_derived' such that we will not rederive the cell again (needed when there are interfaces connected to the cell)
                cell->attributes.erase("\\module_not_derived");
        }
-       // Setting a flag such that it can be known that we have been through all cells at least once, such that we can know whether to flag
-       // an error because of interface instances not found:
+       // Clear the attribute 'cells_not_processed' such that it can be known that we
+       // have been through all cells at least once, and that we can know whether
+       // to flag an error because of interface instances not found:
        module->attributes.erase("\\cells_not_processed");
 
+
+       // If any interface instances were found in the module, we need to rederive it completely:
        if (interfaces_in_module.size() > 0 && !module->get_bool_attribute("\\interfaces_replaced_in_module")) {
                module->reprocess_module(design, interfaces_in_module);
                return did_something;
@@ -736,6 +745,7 @@ struct HierarchyPass : public Pass {
                        }
 
 
+                       // The top module might have changed if interface instances have been detected in it:
                        RTLIL::Module *tmp_top_mod = check_if_top_has_changed(design, top_mod);
                        if (tmp_top_mod != NULL) {
                                if (tmp_top_mod != top_mod){
@@ -744,6 +754,7 @@ struct HierarchyPass : public Pass {
                                }
                        }
 
+                       // Delete modules marked as 'to_delete':
                        std::vector<RTLIL::Module *> modules_to_delete;
                        for(auto &mod_it : design->modules_) {
                                if (mod_it.second->get_bool_attribute("\\to_delete")) {