Fix literals cast to Clocks in Print and Stop

Many tools don't except 'always @(posedge 1'h0)' so we assign the
literal to a wire and use that as the posedge target.

3 files changed, 137 insertions, 0 deletions

diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala
index 67921a42..2d98cf04 100644
--- a/src/main/scala/firrtl/Emitter.scala
+++ b/src/main/scala/firrtl/Emitter.scala
@@ -977,6 +977,7 @@ class VerilogEmitter extends SeqTransform with Emitter {
     new ReplaceTruncatingArithmetic,
     new InlineNotsTransform,
     new InlineCastsTransform,
+    new LegalizeClocksTransform,
     new FlattenRegUpdate,
     new DeadCodeElimination,
     passes.VerilogModulusCleanup,
diff --git a/src/main/scala/firrtl/transforms/LegalizeClocks.scala b/src/main/scala/firrtl/transforms/LegalizeClocks.scala
new file mode 100644
index 00000000..1c2fc045
--- /dev/null
+++ b/src/main/scala/firrtl/transforms/LegalizeClocks.scala
@@ -0,0 +1,69 @@
+package firrtl
+package transforms
+
+import firrtl.ir._
+import firrtl.Mappers._
+import firrtl.Utils.isCast
+
+// Fixup otherwise legal Verilog that lint tools and other tools don't like
+// Currently:
+//   - don't emit "always @(posedge <literal>)"
+//     Hitting this case is rare, but legal FIRRTL
+// TODO This should be unified with all Verilog legalization transforms
+object LegalizeClocksTransform {
+
+  // Checks if an Expression is illegal in use in a @(posedge <Expression>) construct
+  // Legality is defined here by what standard lint tools accept
+  // Currently only looks for literals nested within casts
+  private def illegalClockExpr(expr: Expression): Boolean = expr match {
+    case _: Literal => true
+    case DoPrim(op, args, _,_) if isCast(op) => args.exists(illegalClockExpr)
+    case _ => false
+  }
+
+  /** Legalize Clocks in a Statement
+    *
+    * Enforces legal Verilog semantics on all Clock Expressions.
+    * Legal is defined as what standard lint tools accept.
+    * Currently only Literal Expressions (guarded by casts) are handled.
+    *
+    * @note namespace is lazy because it should not typically be needed
+    */
+  def onStmt(namespace: => Namespace)(stmt: Statement): Statement =
+    stmt.map(onStmt(namespace)) match {
+      // Proper union types would deduplicate this code
+      case r: DefRegister if illegalClockExpr(r.clock) =>
+        val node = DefNode(r.info, namespace.newTemp, r.clock)
+        val rx = r.copy(clock = WRef(node))
+        Block(Seq(node, rx))
+      case p: Print if illegalClockExpr(p.clk) =>
+        val node = DefNode(p.info, namespace.newTemp, p.clk)
+        val px = p.copy(clk = WRef(node))
+        Block(Seq(node, px))
+      case s: Stop if illegalClockExpr(s.clk) =>
+        val node = DefNode(s.info, namespace.newTemp, s.clk)
+        val sx = s.copy(clk = WRef(node))
+        Block(Seq(node, sx))
+      case other => other
+    }
+
+  def onMod(mod: DefModule): DefModule = {
+    // It's actually *extremely* important that this Namespace is a lazy val
+    // onStmt accepts it lazily so that we don't perform the namespacing traversal unless necessary
+    // If we were to inline the declaration, it would create a Namespace for every problem, causing
+    // name collisions
+    lazy val namespace = Namespace(mod)
+    mod.map(onStmt(namespace))
+  }
+}
+
+/** Ensure Clocks to be emitted are legal Verilog */
+class LegalizeClocksTransform extends Transform {
+  def inputForm = LowForm
+  def outputForm = LowForm
+
+  def execute(state: CircuitState): CircuitState = {
+    val modulesx = state.circuit.modules.map(LegalizeClocksTransform.onMod(_))
+    state.copy(circuit = state.circuit.copy(modules = modulesx))
+  }
+}
diff --git a/src/test/scala/firrtlTests/transforms/LegalizeClocks.scala b/src/test/scala/firrtlTests/transforms/LegalizeClocks.scala
new file mode 100644
index 00000000..5c2412ae
--- /dev/null
+++ b/src/test/scala/firrtlTests/transforms/LegalizeClocks.scala
@@ -0,0 +1,67 @@
+// See LICENSE for license details.
+
+package firrtlTests.transforms
+
+import firrtl._
+import firrtlTests.FirrtlFlatSpec
+import firrtlTests.FirrtlCheckers._
+
+class LegalizeClocksTransformSpec extends FirrtlFlatSpec {
+  def compile(input: String): CircuitState =
+    (new MinimumVerilogCompiler).compileAndEmit(CircuitState(parse(input), ChirrtlForm), Nil)
+
+  behavior of "LegalizeClocksTransform"
+
+  it should "not emit @(posedge 1'h0) for stop" in {
+    val input =
+      """circuit test :
+        |  module test :
+        |    stop(asClock(UInt(1)), UInt(1), 1)
+        |""".stripMargin
+    val result = compile(input)
+    result should containLine (s"always @(posedge _GEN_0) begin")
+    result.getEmittedCircuit.value shouldNot include ("always @(posedge 1")
+  }
+
+  it should "not emit @(posedge 1'h0) for printf" in {
+    val input =
+      """circuit test :
+        |  module test :
+        |    printf(asClock(UInt(1)), UInt(1), "hi")
+        |""".stripMargin
+    val result = compile(input)
+    result should containLine (s"always @(posedge _GEN_0) begin")
+    result.getEmittedCircuit.value shouldNot include ("always @(posedge 1")
+  }
+
+  it should "not emit @(posedge 1'h0) for reg" in {
+    val input =
+      """circuit test :
+        |  module test :
+        |    output out : UInt<8>
+        |    input in : UInt<8>
+        |    reg r : UInt<8>, asClock(UInt(0))
+        |    r <= in
+        |    out <= r
+        |""".stripMargin
+    val result = compile(input)
+    result should containLine (s"always @(posedge _GEN_0) begin")
+    result.getEmittedCircuit.value shouldNot include ("always @(posedge 1")
+  }
+
+  it should "deduplicate injected nodes for literal clocks" in {
+    val input =
+      """circuit test :
+        |  module test :
+        |    printf(asClock(UInt(1)), UInt(1), "hi")
+        |    stop(asClock(UInt(1)), UInt(1), 1)
+        |""".stripMargin
+    val result = compile(input)
+    result should containLine (s"wire _GEN_0;")
+    // Check that there's only 1 _GEN_0 instantiation
+    val verilog = result.getEmittedCircuit.value
+    val matches = "wire\\s+_GEN_0;".r.findAllIn(verilog)
+    matches.size should be (1)
+
+  }
+}