From 95d907bd87da1f339264633f12d40673aa7e2818 Mon Sep 17 00:00:00 2001
From: Schuyler Eldridge
Date: Fri, 5 Oct 2018 00:20:10 -0400
Subject: Verilog renaming uses "_", works on whole AST

Summary of changes to firrtl.passes.VerilogRename:
  - Use "_" to mangle names that conflict with Verilog
    keywords (previously "$")
  - Rewrite to operate on the whole AST to propogate mangled ports and
    module names
  - Make VerilogRename a Transform (was previously a Pass)
  - Renames are now propagated
  - Adds documentation for new VerilogRename

This makes the VerilogRename Transform (previously a Pass) use an
underscore ('_') instead of a dollar sign ('$') to mangle names that
conflict with Verilog keywords. This prevents problems with potentially
buggy tools that are not expecting '$' in Verilog names.

This reimplements VerilogRename to be safe for name collisions that may
occur anywhere in the AST, e.g., in ports, module names, circuit names, or
in any statements/expressions. Previously, names were only mangled in
statements and in place. This resulted in problems where renames of ports
in a child's namespace would not be guaranteed to be mangled the same way
in a parent's namespace. The algorithm is reimplemented to walk all
modules in reverse topological order (from leafs to main) and relying on a
RenameMap to track name changes.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
---
 .../scala/firrtlTests/VerilogEmitterTests.scala    | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

(limited to 'src/test')

diff --git a/src/test/scala/firrtlTests/VerilogEmitterTests.scala b/src/test/scala/firrtlTests/VerilogEmitterTests.scala
index b5ad2f1a..5bd17ac9 100644
--- a/src/test/scala/firrtlTests/VerilogEmitterTests.scala
+++ b/src/test/scala/firrtlTests/VerilogEmitterTests.scala
@@ -232,6 +232,43 @@ class VerilogEmitterSpec extends FirrtlFlatSpec {
     }
   }
 
+  "Verilog name conflicts" should "be resolved" in {
+    val input =
+      """|circuit parameter:
+         |  module parameter:
+         |    input always: UInt<1>
+         |    output always$: UInt<1>
+         |    inst assign of endmodule
+         |    node always_ = not(always)
+         |    node always__ = and(always_, assign.fork)
+         |    always$ <= always__
+         |  module endmodule:
+         |    output fork: UInt<1>
+         |    node const = add(UInt<4>("h1"), UInt<3>("h2"))
+         |    fork <= const
+         |""".stripMargin
+    val check_firrtl =
+      """|circuit parameter_:
+         |  module parameter_:
+         |    input always___: UInt<1>
+         |    output always$: UInt<1>
+         |    inst assign_ of endmodule_
+         |    node always_ = not(always___)
+         |    node always__ = and(always_, assign_.fork_)
+         |    always$ <= always__
+         |  module endmodule_:
+         |    output fork_: UInt<1>
+         |    node const_ = add(UInt<4>("h1"), UInt<3>("h2"))
+         |    fork_ <= const_
+         |""".stripMargin
+    val state = CircuitState(parse(input), UnknownForm, Seq.empty, None)
+    val output = Seq( ToWorkingIR, ResolveKinds, InferTypes, VerilogRename )
+      .foldLeft(state){ case (c, tx) => tx.runTransform(c) }
+    Seq( CheckHighForm )
+      .foldLeft(output.circuit){ case (c, tx) => tx.run(c) }
+    output.circuit.serialize should be (parse(check_firrtl).serialize)
+  }
+
 }
 
 class VerilogDescriptionEmitterSpec extends FirrtlFlatSpec {
-- 
cgit v1.2.3


From d426eb766a6177a3488da36ec380df47610c483a Mon Sep 17 00:00:00 2001
From: Schuyler Eldridge
Date: Fri, 5 Oct 2018 13:12:17 -0400
Subject: Refactor VerilogRename -> RemoveKeywordCollisions

This moves VerilogRename out of Passes.scala and genericizes it as the new
Transform KemoveKeywordCollisions. This new Transform will remove keywords
for arbitrary sets of reserved keyword.

This adds VerilogRename back as a class instead of an object.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
---
 src/test/scala/firrtlTests/VerilogEmitterTests.scala | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'src/test')

diff --git a/src/test/scala/firrtlTests/VerilogEmitterTests.scala b/src/test/scala/firrtlTests/VerilogEmitterTests.scala
index 5bd17ac9..3b9f4702 100644
--- a/src/test/scala/firrtlTests/VerilogEmitterTests.scala
+++ b/src/test/scala/firrtlTests/VerilogEmitterTests.scala
@@ -9,6 +9,7 @@ import firrtl._
 import firrtl.annotations._
 import firrtl.ir.Circuit
 import firrtl.passes._
+import firrtl.transforms.VerilogRename
 import firrtl.Parser.IgnoreInfo
 import FirrtlCheckers._
 
@@ -262,7 +263,7 @@ class VerilogEmitterSpec extends FirrtlFlatSpec {
          |    fork_ <= const_
          |""".stripMargin
     val state = CircuitState(parse(input), UnknownForm, Seq.empty, None)
-    val output = Seq( ToWorkingIR, ResolveKinds, InferTypes, VerilogRename )
+    val output = Seq( ToWorkingIR, ResolveKinds, InferTypes, new VerilogRename )
       .foldLeft(state){ case (c, tx) => tx.runTransform(c) }
     Seq( CheckHighForm )
       .foldLeft(output.circuit){ case (c, tx) => tx.run(c) }
-- 
cgit v1.2.3