Use firrtl elses in elsewhen/otherwise case emission (#510)

Preprocess chisel3 IR before emission to determing whether
whens have alternatives.

4 files changed, 90 insertions, 23 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/When.scala b/chiselFrontend/src/main/scala/chisel3/core/When.scala
index 26f939ba..55012946 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/When.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/When.scala
@@ -27,35 +27,52 @@ object when {  // scalastyle:ignore object.name
     * }
     * }}}
     */
-  def apply(cond: Bool)(block: => Unit)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): WhenContext = {
-    new WhenContext(sourceInfo, cond, !cond, block)
+
+  def apply(cond: => Bool)(block: => Unit)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): WhenContext = {
+    new WhenContext(sourceInfo, Some(() => cond), block)
   }
 }
-
-/** Internal mechanism for generating a when. Because of the way FIRRTL
-  * commands are emitted, generating a FIRRTL elsewhen or nested whens inside
-  * elses would be difficult. Instead, this keeps track of the negative of the
-  * previous conditions, so when an elsewhen or otherwise is used, it checks
-  * that both the condition is true and all the previous conditions have been
-  * false.
+ 
+/**  A WhenContext may represent a when, and elsewhen, or an
+  *  otherwise. Since FIRRTL does not have an "elsif" statement,
+  *  alternatives must be mapped to nested if-else statements inside
+  *  the alternatives of the preceeding condition. In order to emit
+  *  proper FIRRTL, it is necessary to keep track of the depth of
+  *  nesting of the FIRRTL whens. Due to the "thin frontend" nature of
+  *  Chisel3, it is not possible to know if a when or elsewhen has a
+  *  succeeding elsewhen or otherwise; therefore, this information is
+  *  added by preprocessing the command queue.
   */
-final class WhenContext(sourceInfo: SourceInfo, cond: Bool, prevCond: => Bool, block: => Unit) {
-  /** This block of logic gets executed if above conditions have been false
-    * and this condition is true.
+final class WhenContext(sourceInfo: SourceInfo, cond: Option[() => Bool], block: => Unit, firrtlDepth: Int = 0) {
+
+  /** This block of logic gets executed if above conditions have been
+    * false and this condition is true. The lazy argument pattern
+    * makes it possible to delay evaluation of cond, emitting the
+    * declaration and assignment of the Bool node of the predicate in
+    * the correct place.
     */
-  def elsewhen (elseCond: Bool)(block: => Unit)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): WhenContext = {
-    new WhenContext(sourceInfo, prevCond && elseCond, prevCond && !elseCond, block)
+  def elsewhen (elseCond: => Bool)(block: => Unit)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): WhenContext = {
+    new WhenContext(sourceInfo, Some(() => elseCond), block, firrtlDepth+1)
   }
 
-  /** This block of logic gets executed only if the above conditions were all
-    * false. No additional logic blocks may be appended past the `otherwise`.
+  /** This block of logic gets executed only if the above conditions
+    * were all false. No additional logic blocks may be appended past
+    * the `otherwise`. The lazy argument pattern makes it possible to
+    * delay evaluation of cond, emitting the declaration and
+    * assignment of the Bool node of the predicate in the correct
+    * place.
     */
   def otherwise(block: => Unit)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Unit =
-    new WhenContext(sourceInfo, prevCond, null, block)
+    new WhenContext(sourceInfo, None, block, firrtlDepth+1)
 
-  pushCommand(WhenBegin(sourceInfo, cond.ref))
+  /*
+   * 
+   */
+  if (firrtlDepth > 0) { pushCommand(AltBegin(sourceInfo)) }
+  cond.foreach( c => pushCommand(WhenBegin(sourceInfo, c().ref)) )
   Builder.whenDepth += 1
   block
   Builder.whenDepth -= 1
-  pushCommand(WhenEnd(sourceInfo))
+  cond.foreach( c => pushCommand(WhenEnd(sourceInfo,firrtlDepth)) )
+  if (cond.isEmpty) { pushCommand(OtherwiseEnd(sourceInfo,firrtlDepth)) }
 }
diff --git a/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala b/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala
index 05cde071..b499c2b1 100644
--- a/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala
+++ b/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala
@@ -260,7 +260,9 @@ case class DefSeqMemory(sourceInfo: SourceInfo, id: HasId, t: Data, size: Int) e
 case class DefMemPort[T <: Data](sourceInfo: SourceInfo, id: T, source: Node, dir: MemPortDirection, index: Arg, clock: Arg) extends Definition
 case class DefInstance(sourceInfo: SourceInfo, id: BaseModule, ports: Seq[Port]) extends Definition
 case class WhenBegin(sourceInfo: SourceInfo, pred: Arg) extends Command
-case class WhenEnd(sourceInfo: SourceInfo) extends Command
+case class WhenEnd(sourceInfo: SourceInfo, firrtlDepth: Int, hasAlt: Boolean = false) extends Command
+case class AltBegin(sourceInfo: SourceInfo) extends Command
+case class OtherwiseEnd(sourceInfo: SourceInfo, firrtlDepth: Int) extends Command
 case class Connect(sourceInfo: SourceInfo, loc: Node, exp: Arg) extends Command
 case class BulkConnect(sourceInfo: SourceInfo, loc1: Node, loc2: Node) extends Command
 case class Attach(sourceInfo: SourceInfo, locs: Seq[Node]) extends Command
diff --git a/src/main/scala/chisel3/internal/firrtl/Emitter.scala b/src/main/scala/chisel3/internal/firrtl/Emitter.scala
index 963a713b..09984722 100644
--- a/src/main/scala/chisel3/internal/firrtl/Emitter.scala
+++ b/src/main/scala/chisel3/internal/firrtl/Emitter.scala
@@ -75,10 +75,21 @@ private class Emitter(circuit: Circuit) {
       case e: DefInvalid => s"${e.arg.fullName(ctx)} is invalid"
       case e: DefInstance => s"inst ${e.name} of ${e.id.name}"
       case w: WhenBegin =>
+        // When consequences are always indented
         indent()
         s"when ${w.pred.fullName(ctx)} :"
-      case _: WhenEnd =>
+      case w: WhenEnd =>
+        // If a when has no else, the indent level must be reset to the enclosing block
         unindent()
+        if (!w.hasAlt) { for (i <- 0 until w.firrtlDepth) { unindent() } }
+        s"skip"
+      case a: AltBegin =>
+        // Else blocks are always indented
+        indent()
+        s"else :"
+      case o: OtherwiseEnd =>
+        // Chisel otherwise: ends all FIRRTL associated a Chisel when, resetting indent level
+        for (i <- 0 until o.firrtlDepth) { unindent() }
         s"skip"
     }
     firrtlLine + e.sourceInfo.makeMessage(" " + _)
@@ -120,8 +131,10 @@ private class Emitter(circuit: Circuit) {
           // Firrtl extmodule can overrule name
           body ++= newline + s"defname = ${bb.id.desiredName}"
           body ++= newline + (bb.params map { case (n, p) => emitParam(n, p) } mkString newline)
-        case mod: DefModule => for (cmd <- mod.commands) {
-          body ++= newline + emit(cmd, mod)
+        case mod: DefModule => {
+          // Preprocess whens & elsewhens, marking those that have no alternative
+          val procMod = mod.copy(commands = processWhens(mod.commands))
+          for (cmd <- procMod.commands) { body ++= newline + emit(cmd, procMod)}
         }
       }
       body ++= newline
@@ -141,6 +154,16 @@ private class Emitter(circuit: Circuit) {
     sb.result
   }
 
+  /** Preprocess the command queue, marking when/elsewhen statements
+    * that have no alternatives (elsewhens or otherwise). These
+    * alternative-free statements reset the indent level to the
+    * enclosing block upon emission.
+    */
+  private def processWhens(cmds: Seq[Command]):
+  Seq[Command] = { cmds.zip(cmds.tail).map({ case (a: WhenEnd, b:
+  AltBegin) => a.copy(hasAlt = true) case (a, b) => a }) ++
+  cmds.lastOption }
+
   private var indentLevel = 0
   private def newline = "\n" + ("  " * indentLevel)
   private def indent(): Unit = indentLevel += 1
diff --git a/src/test/scala/chiselTests/When.scala b/src/test/scala/chiselTests/When.scala
index d4491e13..d2646df7 100644
--- a/src/test/scala/chiselTests/When.scala
+++ b/src/test/scala/chiselTests/When.scala
@@ -53,6 +53,28 @@ class OverlappedWhenTester() extends BasicTester {
   }
 }
 
+class NoOtherwiseOverlappedWhenTester() extends BasicTester {
+  val cnt = Counter(4)
+  when(true.B) { cnt.inc() }
+
+  val out = Wire(UInt(3.W))
+  when(cnt.value <= 0.U) {
+    out := 1.U
+  } .elsewhen (cnt.value <= 1.U) {
+    out := 2.U
+  } .elsewhen (cnt.value <= 2.U) {
+    out := 3.U
+  } .elsewhen (cnt.value <= 3.U) {
+    out := 0.U
+  }
+
+  assert(out === cnt.value + 1.U)
+
+  when(cnt.value === 3.U) {
+    stop()
+  }
+}
+
 class WhenSpec extends ChiselFlatSpec {
   "When, elsewhen, and otherwise with orthogonal conditions" should "work" in {
     assertTesterPasses{ new WhenTester }
@@ -60,4 +82,7 @@ class WhenSpec extends ChiselFlatSpec {
   "When, elsewhen, and otherwise with overlapped conditions" should "work" in {
     assertTesterPasses{ new OverlappedWhenTester }
   }
+  "When and elsewhen without otherwise with overlapped conditions" should "work" in {
+    assertTesterPasses{ new NoOtherwiseOverlappedWhenTester }
+  }
 }