Emitter: add missing parenthesize calls (#1874)

3 files changed, 34 insertions, 5 deletions

diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala
index fe1397a6..397956d8 100644
--- a/src/main/scala/firrtl/Emitter.scala
+++ b/src/main/scala/firrtl/Emitter.scala
@@ -547,12 +547,12 @@ class VerilogEmitter extends SeqTransform with Emitter {
       // This simplifies handling of assignment of a signed expression to an unsigned LHS value
       //   which does not require a cast in Verilog
       case AsUInt | AsSInt | AsClock | AsAsyncReset => Seq(a0)
-      case Dshlw                                    => Seq(cast(a0), " << ", a1)
-      case Dshl                                     => Seq(cast(a0), " << ", a1)
+      case Dshlw                                    => Seq(cast(a0), " << ", parenthesize(a1, false))
+      case Dshl                                     => Seq(cast(a0), " << ", parenthesize(a1, false))
       case Dshr =>
         doprim.tpe match {
-          case (_: SIntType) => Seq(cast(a0), " >>> ", a1)
-          case (_) => Seq(cast(a0), " >> ", a1)
+          case (_: SIntType) => Seq(cast(a0), " >>> ", parenthesize(a1, false))
+          case (_) => Seq(cast(a0), " >> ", parenthesize(a1, false))
         }
       case Shl => if (c0 > 0) Seq("{", cast(a0), s", $c0'h0}") else Seq(cast(a0))
       case Shr if c0 >= bitWidth(a0.tpe) =>
@@ -579,7 +579,7 @@ class VerilogEmitter extends SeqTransform with Emitter {
       case Bits                                                        => Seq(parenthesize(a0, true), "[", c0, ":", c1, "]")
       // If selecting zeroth bit and single-bit wire, just emit the wire
       case Head if c0 == 1 && bitWidth(a0.tpe) == BigInt(1) => Seq(a0)
-      case Head if c0 == 1                                  => Seq(a0, "[", bitWidth(a0.tpe) - 1, "]")
+      case Head if c0 == 1                                  => Seq(parenthesize(a0, true), "[", bitWidth(a0.tpe) - 1, "]")
       case Head =>
         val msb = bitWidth(a0.tpe) - 1
         val lsb = bitWidth(a0.tpe) - c0
diff --git a/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala b/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
index c02b8dd5..31d69cc4 100644
--- a/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
+++ b/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
@@ -8,6 +8,7 @@ import firrtl.annotations.TargetToken.{fromStringToTargetToken, OfModule, Ref}
 import firrtl.ir._
 import firrtl.passes.{InferTypes, LowerTypes, SplitExpressions}
 import firrtl.options.Dependency
+import firrtl.stage.Forms
 import firrtl.PrimOps._
 import firrtl.WrappedExpression._
 
@@ -42,6 +43,8 @@ class InlineBooleanExpressions extends Transform with DependencyAPIMigration {
     Dependency(SplitExpressions)
   )
 
+  override def optionalPrerequisiteOf = Forms.BackendEmitters
+
   override def invalidates(a: Transform) = a match {
     case _: DeadCodeElimination => true // this transform does not remove nodes that are unused after inlining
     case _ => false
diff --git a/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala b/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
index b074e712..72fcff1c 100644
--- a/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
+++ b/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
@@ -242,6 +242,32 @@ class InlineBooleanExpressionsSpec extends FirrtlFlatSpec {
     firrtlEquivalenceTest(input, Seq(new InlineBooleanExpressions))
   }
 
+  it should "emit parentheses in the correct places" in {
+    // should fail if any of these sub-expressions does not have parentheses
+    val input =
+      """
+        |circuit TestParentheses :
+        |  module TestParentheses :
+        |    input in : UInt<1>[3]
+        |    output out : UInt<1>[13]
+        |
+        |    out[0] <= mul(and(in[0], in[1]), in[2])
+        |    out[1] <= div(and(in[0], in[1]), in[2])
+        |    out[2] <= rem(and(in[0], in[1]), in[2])
+        |    out[3] <= add(and(in[0], in[1]), in[2])
+        |    out[4] <= sub(and(in[0], in[1]), in[2])
+        |    out[5] <= dshl(in[0], and(in[1], in[2]))
+        |    out[6] <= dshr(in[0], and(in[1], in[2]))
+        |    out[7] <= lt(and(in[0], in[1]), in[2])
+        |    out[8] <= gt(in[0], or(in[1], in[2]))
+        |    out[9] <= eq(in[0], or(in[1], in[2]))
+        |    out[10] <= neq(in[0], or(in[1], in[2]))
+        |    out[11] <= and(in[0], xor(in[1], in[2]))
+        |    out[12] <= xor(in[0], or(in[1], in[2]))
+    """.stripMargin
+    firrtlEquivalenceTest(input, Seq(new InlineBooleanExpressions))
+  }
+
   it should "avoid inlining when it would create context-sensitivity bugs" in {
     val input =
       """circuit AddNot: