Merge pull request #845 from rdaly525/inline-wires

Preserve Intermediate Wires
rdaly525 · Feb 25, 2020 · f25f195 · f25f195
2 parents 27540ce + c2445b6
commit f25f195
Show file tree

Hide file tree

Showing 19 changed files with 544 additions and 35 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -74,3 +74,7 @@ enable_testing()
 add_executable(test_verilog tests/gtest/test_verilog.cpp)
 target_link_libraries(test_verilog gtest_main coreir coreir-commonlib)
 add_test(NAME test_verilog COMMAND test_verilog WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/gtest)
+
+add_executable(test_inline_wires tests/gtest/test_inline_wires.cpp)
+target_link_libraries(test_inline_wires gtest_main coreir coreir-commonlib)
+add_test(NAME test_inline_wires COMMAND test_inline_wires WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests/gtest)
diff --git a/include/coreir/passes/analysis/verilog.h b/include/coreir/passes/analysis/verilog.h
@@ -28,6 +28,9 @@ class Verilog : public InstanceGraphPass {
   // modules. These parametrized modules have been instanced to create coreir
   // modules, but we only need to compile the verilog definition once
   std::set<Generator *> verilog_generators_seen;
+
+  // Keep track of wire primitive instances, we do not inline these
+  std::set<std::string> wires;
 
   void compileModule(Module *module);
 

diff --git a/src/passes/analysis/verilog.cpp b/src/passes/analysis/verilog.cpp
@@ -149,11 +149,16 @@ bool can_inline_unary_op(CoreIR::Module *module, bool _inline) {
 
 std::unique_ptr<vAST::StructuralStatement> inline_unary_op(
     std::pair<std::string, CoreIR::Instance *> instance,
-    std::unique_ptr<vAST::Connections> verilog_connections) {
-  UnaryOpReplacer transformer(verilog_connections->at("in"));
-  return std::make_unique<vAST::ContinuousAssign>(
-      std::make_unique<vAST::Identifier>(instance.first + "_out"),
-      transformer.visit(get_primitive_expr(instance.second)));
+    std::unique_ptr<vAST::Connections> verilog_connections,
+    bool is_wire) {
+    UnaryOpReplacer transformer(verilog_connections->at("in"));
+    std::string wire_name = instance.first;
+    if (!is_wire) {
+        wire_name += "_out";
+    }
+    return std::make_unique<vAST::ContinuousAssign>(
+        std::make_unique<vAST::Identifier>(wire_name),
+        transformer.visit(get_primitive_expr(instance.second)));
 }
 
 bool can_inline_const_op(CoreIR::Module *module, bool _inline) {
@@ -317,33 +322,50 @@ process_decl(std::unique_ptr<vAST::Identifier> id, Type *type) {
   return std::move(id);
 }
 
+void make_wire_decl(
+    std::string name, Type *type,
+    std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
+                             std::unique_ptr<vAST::Declaration>>>
+        &wire_declarations) {
+    std::unique_ptr<vAST::Identifier> id =
+        std::make_unique<vAST::Identifier>(name);
+    // Can't find a simple way to "convert" a variant type to a
+    // superset, so we just manually unpack it to call the Wire
+    // constructor
+    std::visit(
+        [&](auto &&arg) -> void {
+            wire_declarations.push_back(
+                std::make_unique<vAST::Wire>(std::move(arg)));
+        },
+        process_decl(std::move(id), type));
+}
+
 // Given a map of instances, return a vector of containing declarations for all
 // the output ports of each instance.  We return a vector of a variant type for
 // compatibility with the module AST node constructor, even though we only ever
 // create Wire nodes.
 std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
                          std::unique_ptr<vAST::Declaration>>>
-declare_connections(std::map<std::string, Instance *> instances) {
+declare_connections(std::map<std::string, Instance *> instances, bool _inline) {
   std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
                            std::unique_ptr<vAST::Declaration>>>
       wire_declarations;
   for (auto instance : instances) {
+    if (instance.second->getModuleRef()->getName() == "wire" && _inline) {
+        // Emit inline wire
+        Type *type =
+            cast<RecordType>(instance.second->getModuleRef()->getType())
+                ->getRecord().at("in");
+        make_wire_decl(instance.first, type, wire_declarations);
+        continue;
+    }
     RecordType *record_type =
         cast<RecordType>(instance.second->getModuleRef()->getType());
     for (auto field : record_type->getFields()) {
       Type *field_type = record_type->getRecord().at(field);
       if (!field_type->isInput()) {
-        std::unique_ptr<vAST::Identifier> id =
-            std::make_unique<vAST::Identifier>(instance.first + "_" + field);
-        // Can't find a simple way to "convert" a variant type to a
-        // superset, so we just manually unpack it to call the Wire
-        // constructor
-        std::visit(
-            [&](auto &&arg) -> void {
-              wire_declarations.push_back(
-                  std::make_unique<vAST::Wire>(std::move(arg)));
-            },
-            process_decl(std::move(id), field_type));
+        make_wire_decl(instance.first + "_" + field, field_type,
+                       wire_declarations);
       }
     }
   }
@@ -569,11 +591,18 @@ build_connection_map(CoreIR::ModuleDef *definition,
 
 // Join select path fields by "_" (ignoring intial self if present)
 std::variant<std::unique_ptr<vAST::Identifier>, std::unique_ptr<vAST::Index>>
-convert_to_verilog_connection(Wireable *value) {
+convert_to_verilog_connection(Wireable *value, bool _inline) {
   SelectPath select_path = value->getSelectPath();
   if (select_path.front() == "self") {
     select_path.pop_front();
   }
+  Wireable *parent = value->getTopParent();
+  if (_inline && parent->getKind() == Wireable::WK_Instance &&
+      cast<Instance>(parent)->getModuleRef()->getName() == "wire") {
+    // Use instance name as wire name
+    select_path.pop_front();
+    select_path[0] = parent->toString();
+  }
   std::string connection_name = "";
   for (uint i = 0; i < select_path.size(); i++) {
     auto item = select_path[i];
@@ -612,12 +641,12 @@ std::unique_ptr<vAST::Concat>
 convert_non_bulk_connection_to_concat(std::vector<ConnMapEntry> entries,
         std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
                                  std::unique_ptr<vAST::Declaration>>> &body,
-        std::string debug_prefix) {
+        std::string debug_prefix, bool _inline) {
   std::vector<std::unique_ptr<vAST::Expression>> args;
   args.resize(entries.size());
   for (auto entry : entries) {
     std::unique_ptr<vAST::Expression> verilog_conn =
-        convert_to_expression(convert_to_verilog_connection(entry.source));
+        convert_to_expression(convert_to_verilog_connection(entry.source, _inline));
     process_connection_debug_metadata(entry, verilog_conn->toString(), body,
             debug_prefix + "[" + std::to_string(entry.index) + "]");
     args[entry.index] = std::move(verilog_conn);
@@ -632,7 +661,7 @@ void assign_module_outputs(
     RecordType *record_type,
     std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
                              std::unique_ptr<vAST::Declaration>>> &body,
-    std::map<ConnMapKey, std::vector<ConnMapEntry>> connection_map) {
+    std::map<ConnMapKey, std::vector<ConnMapEntry>> connection_map, bool _inline) {
   for (auto field : record_type->getFields()) {
     Type *field_type = record_type->getRecord().at(field);
     if (field_type->isInput()) {
@@ -643,12 +672,12 @@ void assign_module_outputs(
       continue;
     } else if (entries.size() > 1) {
       std::unique_ptr<vAST::Concat> concat =
-          convert_non_bulk_connection_to_concat(entries, body, field);
+          convert_non_bulk_connection_to_concat(entries, body, field, _inline);
       body.push_back(std::make_unique<vAST::ContinuousAssign>(
           std::make_unique<vAST::Identifier>(field), std::move(concat)));
     } else {
       std::unique_ptr<vAST::Expression> verilog_conn =
-          convert_to_expression(convert_to_verilog_connection(entries[0].source));
+          convert_to_expression(convert_to_verilog_connection(entries[0].source, _inline));
       process_connection_debug_metadata(entries[0], verilog_conn->toString(), body,
               field);
       // Regular (possibly bulk) connection
@@ -664,13 +693,14 @@ void assign_module_outputs(
 void assign_inouts(
     std::vector<Connection> connections,
     std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
-                             std::unique_ptr<vAST::Declaration>>> &body) {
+                             std::unique_ptr<vAST::Declaration>>> &body,
+    bool _inline) {
   for (auto connection : connections) {
     if (connection.first->getType()->isInOut() ||
         connection.second->getType()->isInOut()) {
       body.push_back(std::make_unique<vAST::ContinuousAssign>(
-              convert_to_assign_target(convert_to_verilog_connection(connection.first)),
-              convert_to_expression(convert_to_verilog_connection(connection.second))
+              convert_to_assign_target(convert_to_verilog_connection(connection.first, _inline)),
+              convert_to_expression(convert_to_verilog_connection(connection.second, _inline))
        ));
     };
   };
@@ -683,12 +713,12 @@ std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
                          std::unique_ptr<vAST::Declaration>>>
 compile_module_body(RecordType *module_type,
                     CoreIR::ModuleDef *definition,
-                    bool _inline) {
+                    bool _inline, std::set<std::string> &wires) {
   std::map<std::string, Instance *> instances = definition->getInstances();
 
   std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
                            std::unique_ptr<vAST::Declaration>>>
-      body = declare_connections(instances);
+      body = declare_connections(instances, _inline);
 
   std::map<ConnMapKey, std::vector<ConnMapEntry>> connection_map =
       build_connection_map(definition, instances);
@@ -738,12 +768,12 @@ compile_module_body(RecordType *module_type,
       } else if (entries.size() > 1) {
         std::unique_ptr<vAST::Concat> concat =
             convert_non_bulk_connection_to_concat(entries, body, 
-                instance_name + "." + field);
+                instance_name + "." + field, _inline);
         verilog_connections->insert(field, std::move(concat));
         // Otherwise we just use the entry in the connection map
       } else {
         std::unique_ptr<vAST::Expression> verilog_conn =
-            convert_to_expression(convert_to_verilog_connection(entries[0].source));
+            convert_to_expression(convert_to_verilog_connection(entries[0].source, _inline));
         process_connection_debug_metadata(entries[0], verilog_conn->toString(), body,
                 instance_name + "." + field);
         verilog_connections->insert(field, std::move(verilog_conn));
@@ -772,7 +802,12 @@ compile_module_body(RecordType *module_type,
     if (can_inline_binary_op(instance_module, _inline)) {
         statement = inline_binary_op(instance, std::move(verilog_connections));
     } else if (can_inline_unary_op(instance_module, _inline)) {
-        statement = inline_unary_op(instance, std::move(verilog_connections));
+        bool is_wire = instance_module->getName() == "wire";
+        if (is_wire) {
+            wires.insert(instance.first);
+        }
+        statement = inline_unary_op(instance, std::move(verilog_connections),
+                                    is_wire);
     } else if (can_inline_mux_op(instance_module, _inline)) {
         statement = inline_mux_op(instance, std::move(verilog_connections));
     } else if (can_inline_const_op(instance_module, _inline)) {
@@ -805,8 +840,10 @@ compile_module_body(RecordType *module_type,
     body.push_back(std::move(statement));
   }
   // Wire the outputs of the module and inout connections
-  assign_module_outputs(module_type, body, connection_map);
-  assign_inouts(definition->getSortedConnections(), body);
+  // TODO: Make these object methods so we don't have to pass things aorund so
+  // much (e.g. _inline flag)
+  assign_module_outputs(module_type, body, connection_map, _inline);
+  assign_inouts(definition->getSortedConnections(), body, _inline);
   return body;
 }
 
@@ -895,7 +932,7 @@ void Passes::Verilog::compileModule(Module *module) {
       body;
   if (module->hasDef()) {
       body = compile_module_body(module->getType(), definition,
-                                 this->_inline);
+                                 this->_inline, this->wires);
   }
 
   if (module->getMetaData().count("filename") > 0) { 
@@ -925,7 +962,7 @@ void Passes::Verilog::compileModule(Module *module) {
                                      std::move(parameters));
 
   if (this->_inline) {
-      vAST::AssignInliner transformer;
+      vAST::AssignInliner transformer(this->wires);
       verilog_module = transformer.visit(std::move(verilog_module));
   }
   modules.push_back(std::make_pair(name, std::move(verilog_module)));

diff --git a/tests/gtest/golds/arr_tuple.v b/tests/gtest/golds/arr_tuple.v
@@ -0,0 +1,21 @@
+// Module `Foo` defined externally
+module Main (
+    input z_0_x,
+    output z_0_y,
+    input z_1_x,
+    output z_1_y
+);
+wire Foo_inst0_z_0_y;
+wire a_x;
+wire a_y;
+Foo Foo_inst0 (
+    .z_0_x(a_y),
+    .z_0_y(Foo_inst0_z_0_y),
+    .z_1_x(z_0_x),
+    .z_1_y(z_1_y)
+);
+assign a_x = Foo_inst0_z_0_y;
+assign a_y = z_0_x;
+assign z_0_y = a_x;
+endmodule
+
diff --git a/tests/gtest/golds/bit_wire.v b/tests/gtest/golds/bit_wire.v
@@ -0,0 +1,9 @@
+module Main (
+    input I,
+    output O
+);
+wire x;
+assign x = I;
+assign O = x;
+endmodule
+
diff --git a/tests/gtest/golds/bits_instance.v b/tests/gtest/golds/bits_instance.v
@@ -0,0 +1,15 @@
+// Module `Foo` defined externally
+module Main (
+    input [4:0] I,
+    output [4:0] O
+);
+wire [4:0] Foo_inst0_O;
+wire [4:0] x;
+Foo Foo_inst0 (
+    .I(I),
+    .O(Foo_inst0_O)
+);
+assign x = Foo_inst0_O;
+assign O = x;
+endmodule
+
diff --git a/tests/gtest/golds/bits_wire.v b/tests/gtest/golds/bits_wire.v
@@ -0,0 +1,9 @@
+module Main (
+    input [4:0] I,
+    output [4:0] O
+);
+wire [4:0] x;
+assign x = I;
+assign O = x;
+endmodule
+
diff --git a/tests/gtest/golds/fanout.v b/tests/gtest/golds/fanout.v
@@ -0,0 +1,11 @@
+module Main (
+    input I,
+    output O0,
+    output O1
+);
+wire x;
+assign x = I;
+assign O0 = x;
+assign O1 = x;
+endmodule
+
diff --git a/tests/gtest/golds/nested_array_wire.v b/tests/gtest/golds/nested_array_wire.v
@@ -0,0 +1,21 @@
+module Main (
+    input [4:0] I_0,
+    input [4:0] I_1,
+    input [4:0] I_2,
+    input [4:0] I_3,
+    input [4:0] I_4,
+    output [4:0] O_0,
+    output [4:0] O_1,
+    output [4:0] O_2,
+    output [4:0] O_3,
+    output [4:0] O_4
+);
+wire [24:0] x;
+assign x = {I_4[4],I_4[3],I_4[2],I_4[1],I_4[0],I_3[4],I_3[3],I_3[2],I_3[1],I_3[0],I_2[4],I_2[3],I_2[2],I_2[1],I_2[0],I_1[4],I_1[3],I_1[2],I_1[1],I_1[0],I_0[4],I_0[3],I_0[2],I_0[1],I_0[0]};
+assign O_0 = {x[4],x[3],x[2],x[1],x[0]};
+assign O_1 = {x[9],x[8],x[7],x[6],x[5]};
+assign O_2 = {x[14],x[13],x[12],x[11],x[10]};
+assign O_3 = {x[19],x[18],x[17],x[16],x[15]};
+assign O_4 = {x[24],x[23],x[22],x[21],x[20]};
+endmodule
+
diff --git a/tests/gtest/golds/tuple.v b/tests/gtest/golds/tuple.v
@@ -0,0 +1,12 @@
+module Main (
+    input [4:0] I__0,
+    input I__1,
+    output [4:0] O__0,
+    output O__1
+);
+wire [5:0] x;
+assign x = {I__1,I__0[4],I__0[3],I__0[2],I__0[1],I__0[0]};
+assign O__0 = {x[4],x[3],x[2],x[1],x[0]};
+assign O__1 = x[5];
+endmodule
+
diff --git a/tests/gtest/srcs/arr_tuple.json b/tests/gtest/srcs/arr_tuple.json
@@ -0,0 +1,37 @@
+{"top":"global.Main",
+"namespaces":{
+  "global":{
+    "modules":{
+      "Foo":{
+        "type":["Record",[
+          ["z",["Array",2,["Record",[["x","BitIn"],["y","Bit"]]]]]
+        ]]
+      },
+      "Main":{
+        "type":["Record",[
+          ["z",["Array",2,["Record",[["x","BitIn"],["y","Bit"]]]]]
+        ]],
+        "instances":{
+          "Foo_inst0":{
+            "modref":"global.Foo"
+          },
+          "a_x":{
+            "modref":"corebit.wire"
+          },
+          "a_y":{
+            "modref":"corebit.wire"
+          }
+        },
+        "connections":[
+          ["a_y.out","Foo_inst0.z.0.x"],
+          ["a_x.in","Foo_inst0.z.0.y"],
+          ["self.z.0.x","Foo_inst0.z.1.x"],
+          ["self.z.1.y","Foo_inst0.z.1.y"],
+          ["self.z.0.y","a_x.out"],
+          ["self.z.0.x","a_y.in"]
+        ]
+      }
+    }
+  }
+}
+}