Add --pretty:no-expr-inlining to prevent expression inlining (#1869)

Also rename --Wno-scala-version-warning to
--warn:no-scala-version-deprecation and adopt naming convention where
resulting annotation matches the CLI option

9 files changed, 101 insertions, 34 deletions

diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala
index 3390e632..56072cb4 100644
--- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala
+++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala
@@ -253,10 +253,10 @@ case class FirrtlCircuitAnnotation(circuit: Circuit) extends NoTargetAnnotation
 
 /** Suppresses warning about Scala 2.11 deprecation
   *
-  *  - set with `--Wno-scala-version-warning`
+  *  - set with `--warn:no-scala-version-deprecation`
   */
-case object SuppressScalaVersionWarning extends NoTargetAnnotation with FirrtlOption with HasShellOptions {
-  def longOption: String = "Wno-scala-version-warning"
+case object WarnNoScalaVersionDeprecation extends NoTargetAnnotation with FirrtlOption with HasShellOptions {
+  def longOption: String = "warn:no-scala-version-deprecation"
   val options = Seq(
     new ShellOption[Unit](
       longOption = longOption,
@@ -265,3 +265,18 @@ case object SuppressScalaVersionWarning extends NoTargetAnnotation with FirrtlOp
     )
   )
 }
+
+/** Turn off all expression inlining
+  *
+  * @note this primarily applies to emitted Verilog
+  */
+case object PrettyNoExprInlining extends NoTargetAnnotation with FirrtlOption with HasShellOptions {
+  def longOption: String = "pretty:no-expr-inlining"
+  val options = Seq(
+    new ShellOption[Unit](
+      longOption = longOption,
+      toAnnotationSeq = { _ => Seq(this) },
+      helpText = "Disable expression inlining"
+    )
+  )
+}
diff --git a/src/main/scala/firrtl/stage/FirrtlCli.scala b/src/main/scala/firrtl/stage/FirrtlCli.scala
index c33980dd..450248ea 100644
--- a/src/main/scala/firrtl/stage/FirrtlCli.scala
+++ b/src/main/scala/firrtl/stage/FirrtlCli.scala
@@ -20,7 +20,8 @@ trait FirrtlCli { this: Shell =>
     firrtl.EmitCircuitAnnotation,
     firrtl.EmitAllModulesAnnotation,
     NoCircuitDedupAnnotation,
-    SuppressScalaVersionWarning
+    WarnNoScalaVersionDeprecation,
+    PrettyNoExprInlining
   )
     .map(_.addOptions(parser))
 
diff --git a/src/main/scala/firrtl/stage/package.scala b/src/main/scala/firrtl/stage/package.scala
index 37e2d13c..21182c84 100644
--- a/src/main/scala/firrtl/stage/package.scala
+++ b/src/main/scala/firrtl/stage/package.scala
@@ -32,7 +32,8 @@ package object stage {
           case InfoModeAnnotation(i)        => c.copy(infoModeName = i)
           case FirrtlCircuitAnnotation(cir) => c.copy(firrtlCircuit = Some(cir))
           case a: CompilerAnnotation => logger.warn(s"Use of CompilerAnnotation is deprecated. Ignoring $a"); c
-          case SuppressScalaVersionWarning => c
+          case WarnNoScalaVersionDeprecation => c
+          case PrettyNoExprInlining          => c
         }
       }
   }
diff --git a/src/main/scala/firrtl/stage/transforms/CheckScalaVersion.scala b/src/main/scala/firrtl/stage/transforms/CheckScalaVersion.scala
index a7127dba..65dfd89a 100644
--- a/src/main/scala/firrtl/stage/transforms/CheckScalaVersion.scala
+++ b/src/main/scala/firrtl/stage/transforms/CheckScalaVersion.scala
@@ -3,7 +3,7 @@
 package firrtl.stage.transforms
 
 import firrtl.{BuildInfo, CircuitState, DependencyAPIMigration, Transform}
-import firrtl.stage.SuppressScalaVersionWarning
+import firrtl.stage.WarnNoScalaVersionDeprecation
 import firrtl.options.StageUtils.dramaticWarning
 
 object CheckScalaVersion {
@@ -27,9 +27,9 @@ class CheckScalaVersion extends Transform with DependencyAPIMigration {
   override def invalidates(a: Transform) = false
 
   def execute(state: CircuitState): CircuitState = {
-    def suppress = state.annotations.contains(SuppressScalaVersionWarning)
+    def suppress = state.annotations.contains(WarnNoScalaVersionDeprecation)
     if (getScalaMajorVersion == 11 && !suppress) {
-      val option = s"--${SuppressScalaVersionWarning.longOption}"
+      val option = s"--${WarnNoScalaVersionDeprecation.longOption}"
       dramaticWarning(deprecationMessage("2.11", option))
     }
     state
diff --git a/src/main/scala/firrtl/transforms/CombineCats.scala b/src/main/scala/firrtl/transforms/CombineCats.scala
index 3014d0e3..6d05698f 100644
--- a/src/main/scala/firrtl/transforms/CombineCats.scala
+++ b/src/main/scala/firrtl/transforms/CombineCats.scala
@@ -7,6 +7,7 @@ import firrtl.PrimOps._
 import firrtl.WrappedExpression._
 import firrtl.annotations.NoTargetAnnotation
 import firrtl.options.Dependency
+import firrtl.stage.PrettyNoExprInlining
 
 import scala.collection.mutable
 
@@ -74,11 +75,18 @@ class CombineCats extends Transform with DependencyAPIMigration {
   val defaultMaxCatLen = 10
 
   def execute(state: CircuitState): CircuitState = {
-    val maxCatLen = state.annotations.collectFirst {
-      case m: MaxCatLenAnnotation => m.maxCatLen
-    }.getOrElse(defaultMaxCatLen)
-
-    val modulesx = state.circuit.modules.map(CombineCats.onMod(maxCatLen))
-    state.copy(circuit = state.circuit.copy(modules = modulesx))
+    val run = !state.annotations.contains(PrettyNoExprInlining)
+
+    if (run) {
+      val maxCatLen = state.annotations.collectFirst {
+        case m: MaxCatLenAnnotation => m.maxCatLen
+      }.getOrElse(defaultMaxCatLen)
+
+      val modulesx = state.circuit.modules.map(CombineCats.onMod(maxCatLen))
+      state.copy(circuit = state.circuit.copy(modules = modulesx))
+    } else {
+      logger.info(s"--${PrettyNoExprInlining.longOption} specified, skipping...")
+      state
+    }
   }
 }
diff --git a/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala b/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
index 31d69cc4..b405f353 100644
--- a/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
+++ b/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
@@ -3,6 +3,7 @@
 package firrtl
 package transforms
 
+import firrtl.stage.PrettyNoExprInlining
 import firrtl.annotations.{NoTargetAnnotation, Target}
 import firrtl.annotations.TargetToken.{fromStringToTargetToken, OfModule, Ref}
 import firrtl.ir._
@@ -159,26 +160,33 @@ class InlineBooleanExpressions extends Transform with DependencyAPIMigration {
   }
 
   def execute(state: CircuitState): CircuitState = {
-    val dontTouchMap: Map[OfModule, Set[Ref]] = {
-      val refTargets = state.annotations.flatMap {
-        case anno: HasDontTouches => anno.dontTouches
-        case o => Nil
-      }
-      val dontTouches: Seq[(OfModule, Ref)] = refTargets.map {
-        case r => Target.referringModule(r).module.OfModule -> r.ref.Ref
+    val run = !state.annotations.contains(PrettyNoExprInlining)
+
+    if (run) {
+      val dontTouchMap: Map[OfModule, Set[Ref]] = {
+        val refTargets = state.annotations.flatMap {
+          case anno: HasDontTouches => anno.dontTouches
+          case o => Nil
+        }
+        val dontTouches: Seq[(OfModule, Ref)] = refTargets.map {
+          case r => Target.referringModule(r).module.OfModule -> r.ref.Ref
+        }
+        dontTouches.groupBy(_._1).mapValues(_.map(_._2).toSet).toMap
       }
-      dontTouches.groupBy(_._1).mapValues(_.map(_._2).toSet).toMap
-    }
 
-    val maxInlineCount = state.annotations.collectFirst {
-      case InlineBooleanExpressionsMax(max) => max
-    }.getOrElse(InlineBooleanExpressions.defaultMax)
+      val maxInlineCount = state.annotations.collectFirst {
+        case InlineBooleanExpressionsMax(max) => max
+      }.getOrElse(InlineBooleanExpressions.defaultMax)
 
-    val modulesx = state.circuit.modules.map { m =>
-      val mapMethods = new MapMethods(maxInlineCount, dontTouchMap.getOrElse(m.name.OfModule, Set.empty[Ref]))
-      m.mapStmt(mapMethods.onStmt(_))
-    }
+      val modulesx = state.circuit.modules.map { m =>
+        val mapMethods = new MapMethods(maxInlineCount, dontTouchMap.getOrElse(m.name.OfModule, Set.empty[Ref]))
+        m.mapStmt(mapMethods.onStmt(_))
+      }
 
-    state.copy(circuit = state.circuit.copy(modules = modulesx))
+      state.copy(circuit = state.circuit.copy(modules = modulesx))
+    } else {
+      logger.info(s"--${PrettyNoExprInlining.longOption} specified, skipping...")
+      state
+    }
   }
 }
diff --git a/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala b/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
index fb7ecfdb..15dffee6 100644
--- a/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
+++ b/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
@@ -8,7 +8,7 @@ import firrtl.options.Dependency
 import firrtl.passes._
 import firrtl.transforms._
 import firrtl.testutils._
-import firrtl.stage.TransformManager
+import firrtl.stage.{PrettyNoExprInlining, TransformManager}
 
 class InlineBooleanExpressionsSpec extends FirrtlFlatSpec {
   val transform = new InlineBooleanExpressions
@@ -312,4 +312,19 @@ class InlineBooleanExpressionsSpec extends FirrtlFlatSpec {
         |    o <= add(a, not(b))""".stripMargin
     firrtlEquivalenceTest(input, Seq(new InlineBooleanExpressions))
   }
+
+  it should s"respect --${PrettyNoExprInlining.longOption}" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    input a : UInt<1>
+        |    input b : UInt<1>
+        |    input c : UInt<1>
+        |    output out : UInt<1>
+        |
+        |    node _T_1 = and(a, b)
+        |    out <= and(_T_1, c)""".stripMargin
+    val result = exec(input, PrettyNoExprInlining :: Nil)
+    (result) should be(parse(input).serialize)
+  }
 }
diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
index ee942d85..bf4b790e 100644
--- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
+++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
@@ -10,7 +10,7 @@ import java.io.{File, PrintWriter}
 
 import firrtl.{BuildInfo, FileUtils}
 
-import firrtl.stage.{FirrtlMain, SuppressScalaVersionWarning}
+import firrtl.stage.{FirrtlMain, WarnNoScalaVersionDeprecation}
 import firrtl.stage.transforms.CheckScalaVersion
 import firrtl.util.BackendCompilationUtilities
 import org.scalatest.featurespec.AnyFeatureSpec
@@ -169,7 +169,7 @@ class FirrtlMainSpec
     */
   val defaultStdOut: Option[String] = BuildInfo.scalaVersion.split("\\.").toList match {
     case "2" :: v :: _ :: Nil if v.toInt <= 11 =>
-      Some(CheckScalaVersion.deprecationMessage("2.11", s"--${SuppressScalaVersionWarning.longOption}"))
+      Some(CheckScalaVersion.deprecationMessage("2.11", s"--${WarnNoScalaVersionDeprecation.longOption}"))
     case x =>
       None
   }
diff --git a/src/test/scala/firrtlTests/transforms/CombineCatsSpec.scala b/src/test/scala/firrtlTests/transforms/CombineCatsSpec.scala
index a916eac5..cd0907dc 100644
--- a/src/test/scala/firrtlTests/transforms/CombineCatsSpec.scala
+++ b/src/test/scala/firrtlTests/transforms/CombineCatsSpec.scala
@@ -5,6 +5,7 @@ package firrtlTests.transforms
 import firrtl.PrimOps._
 import firrtl._
 import firrtl.ir.DoPrim
+import firrtl.stage.PrettyNoExprInlining
 import firrtl.transforms.{CombineCats, MaxCatLenAnnotation}
 import firrtl.testutils.FirrtlFlatSpec
 import firrtl.testutils.FirrtlCheckers._
@@ -174,4 +175,22 @@ class CombineCatsSpec extends FirrtlFlatSpec {
       case DoPrim(Cat, Seq(_, DoPrim(Cat, Seq(_, DoPrim(Cat, _, _, _)), _, _)), _, _) => true
     }
   }
+
+  "CombineCats" should s"respect --${PrettyNoExprInlining.longOption}" in {
+    val input =
+      """circuit test :
+        |  module test :
+        |    input in1 : UInt<1>
+        |    input in2 : UInt<2>
+        |    input in3 : UInt<3>
+        |    input in4 : UInt<4>
+        |    output out : UInt<10>
+        |
+        |    node _T_1 = cat(in1, in2)
+        |    node _T_2 = cat(_T_1, in3)
+        |    out <= cat(_T_2, in4)
+        |""".stripMargin
+    val result = execute(input, transforms, PrettyNoExprInlining :: Nil)
+    result.circuit.serialize should be(parse(input).serialize)
+  }
 }