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