From c3e3edc5b860cccdb754dd5112ea8d8375491796 Mon Sep 17 00:00:00 2001
From: Lenny Truong <leonardtruong@fb.com>
Date: Thu, 30 Jan 2020 17:22:54 -0800
Subject: [PATCH 1/2] Fix verilator_debug inline bug

---
 src/passes/analysis/verilog.cpp             | 14 ++++----
 tests/gtest/test_verilog.cpp                | 24 ++++++++++++++
 tests/gtest/verilator_debug_inline.json     | 36 +++++++++++++++++++++
 tests/gtest/verilator_debug_inline_golden.v |  8 +++++
 4 files changed, 74 insertions(+), 8 deletions(-)
 create mode 100644 tests/gtest/verilator_debug_inline.json
 create mode 100644 tests/gtest/verilator_debug_inline_golden.v

diff --git a/src/passes/analysis/verilog.cpp b/src/passes/analysis/verilog.cpp
index ab04fc12c..21ef81ddd 100644
--- a/src/passes/analysis/verilog.cpp
+++ b/src/passes/analysis/verilog.cpp
@@ -460,17 +460,15 @@ Passes::Verilog::compilePorts(RecordType *record_type) {
     } else {
       ASSERT(false, "Not implemented for type = " + toString(type));
     }
-    std::unique_ptr<vAST::Port> port = std::make_unique<vAST::Port>(
+    std::unique_ptr<vAST::AbstractPort> port = std::make_unique<vAST::Port>(
             process_decl(std::move(name), type), verilog_direction, vAST::WIRE);
     if (this->verilator_debug) {
-      // FIXME: Hack to get comment into port decl, we need to add support
-      // attaching comments to expressions
-      std::string port_str = port->toString();
-      port_str += "/*verilator public*/";
-      ports.push_back(std::make_unique<vAST::StringPort>(port_str));
-    } else {
-      ports.push_back(std::move(port));
+      port = std::make_unique<vAST::PortComment>(
+        std::move(port), 
+        "verilator public"
+      );
     }
+    ports.push_back(std::move(port));
   };
   return ports;
 }
diff --git a/tests/gtest/test_verilog.cpp b/tests/gtest/test_verilog.cpp
index cf2c657a9..b9845cbca 100644
--- a/tests/gtest/test_verilog.cpp
+++ b/tests/gtest/test_verilog.cpp
@@ -206,6 +206,30 @@ TEST(VerilogTests, TestDebugInfo) {
   deleteContext(c);
 }
 
+TEST(VerilogTests, TestVerilatorDebugInline) {
+  Context* c = newContext();
+  CoreIRLoadVerilog_coreir(c);
+  CoreIRLoadVerilog_corebit(c);
+  CoreIRLoadLibrary_commonlib(c);
+  Module* top;
+
+  if (!loadFromFile(c, "verilator_debug_inline.json", &top)) {
+    c->die();
+  }
+  assert(top != nullptr);
+  c->setTop(top->getRefName());
+
+  const std::vector<std::string> passes = {
+    "rungenerators",
+    "removebulkconnections",
+    "flattentypes",
+    "verilog --inline --verilator_debug"
+  };
+  c->runPasses(passes, {});
+  assertPassEq<Passes::Verilog>(c, "verilator_debug_inline_golden.v");
+  deleteContext(c);
+}
+
 TEST(VerilogTests, TestRegisterMode) {
   Context* c = newContext();
   CoreIRLoadVerilog_coreir(c);
diff --git a/tests/gtest/verilator_debug_inline.json b/tests/gtest/verilator_debug_inline.json
new file mode 100644
index 000000000..3f32186bc
--- /dev/null
+++ b/tests/gtest/verilator_debug_inline.json
@@ -0,0 +1,36 @@
+{"top":"global.Top",
+"namespaces":{
+  "global":{
+    "modules":{
+      "Top":{
+        "type":["Record",[
+          ["CLK",["Named","coreir.clkIn"]],
+          ["O",["Record",[["A","Bit"]]]]
+        ]],
+        "instances":{
+          "Top_comb_inst0":{
+            "modref":"global.Top_comb"
+          }
+        },
+        "connections":[
+          ["self.O","Top_comb_inst0.O"]
+        ]
+      },
+      "Top_comb":{
+        "type":["Record",[
+          ["O",["Record",[["A","Bit"]]]]
+        ]],
+        "instances":{
+          "bit_const_0_None":{
+            "modref":"corebit.const",
+            "modargs":{"value":["Bool",false]}
+          }
+        },
+        "connections":[
+          ["self.O.A","bit_const_0_None.out"]
+        ]
+      }
+    }
+  }
+}
+}
diff --git a/tests/gtest/verilator_debug_inline_golden.v b/tests/gtest/verilator_debug_inline_golden.v
new file mode 100644
index 000000000..a5a23db45
--- /dev/null
+++ b/tests/gtest/verilator_debug_inline_golden.v
@@ -0,0 +1,8 @@
+module Top_comb (output O_A/*verilator public*/);
+assign O_A = 1'b0;
+endmodule
+
+module Top (input CLK/*verilator public*/, output O_A/*verilator public*/);
+Top_comb Top_comb_inst0(.O_A(O_A));
+endmodule
+

From fe3963d18bba52f9529c9b698ddf61ec0b708564 Mon Sep 17 00:00:00 2001
From: Lenny Truong <leonardtruong@fb.com>
Date: Mon, 3 Feb 2020 17:08:36 -0800
Subject: [PATCH 2/2] Update to WithComment

---
 src/passes/analysis/verilog.cpp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/passes/analysis/verilog.cpp b/src/passes/analysis/verilog.cpp
index 21ef81ddd..85ea56a57 100644
--- a/src/passes/analysis/verilog.cpp
+++ b/src/passes/analysis/verilog.cpp
@@ -460,13 +460,11 @@ Passes::Verilog::compilePorts(RecordType *record_type) {
     } else {
       ASSERT(false, "Not implemented for type = " + toString(type));
     }
-    std::unique_ptr<vAST::AbstractPort> port = std::make_unique<vAST::Port>(
-            process_decl(std::move(name), type), verilog_direction, vAST::WIRE);
+    std::unique_ptr<vAST::Port> port = std::make_unique<vAST::Port>(
+            process_decl(std::move(name), type), verilog_direction,
+            vAST::WIRE);
     if (this->verilator_debug) {
-      port = std::make_unique<vAST::PortComment>(
-        std::move(port), 
-        "verilator public"
-      );
+      port = vAST::AddComment(std::move(port), "verilator public");
     }
     ports.push_back(std::move(port));
   };