Merge pull request #1875 from freechipsproject/fix-inline-bools

Restrict boolean inlining to avoid context-sensitive width bugs

2 files changed, 28 insertions, 5 deletions

diff --git a/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala b/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
index 7c52d6ef..c02b8dd5 100644
--- a/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
+++ b/src/main/scala/firrtl/transforms/InlineBooleanExpressions.scala
@@ -83,11 +83,22 @@ class InlineBooleanExpressions extends Transform with DependencyAPIMigration {
 
     /** Whether or not an can be inlined
       * @param refExpr the expression to check for inlining
+      * @param outerExpr the parent expression of refExpr, if any
       */
-    def canInline(refExpr: Expression): Boolean = {
-      refExpr match {
-        case _: Mux => false
-        case _ => refExpr.tpe == Utils.BoolType
+    def canInline(refExpr: Expression, outerExpr: Option[Expression]): Boolean = {
+      val contextInsensitiveDetOps: Set[PrimOp] = Set(Lt, Leq, Gt, Geq, Eq, Neq, Andr, Orr, Xorr)
+      outerExpr match {
+        case None => true
+        case Some(o) if (o.tpe == Utils.BoolType) =>
+          refExpr match {
+            case _: Mux => false
+            case e => e.tpe == Utils.BoolType
+          }
+        case Some(o) =>
+          refExpr match {
+            case DoPrim(op, _, _, Utils.BoolType) => contextInsensitiveDetOps(op)
+            case _                                => false
+          }
       }
     }
 
@@ -105,7 +116,8 @@ class InlineBooleanExpressions extends Transform with DependencyAPIMigration {
           netlist.get(we(ref)) match {
             case Some((refExpr, refInfo)) if sameFileAndLineInfo(info, refInfo) =>
               val inlineNum = inlineCounts.getOrElse(refKey, 1)
-              if (!outerExpr.isDefined || canInline(refExpr) && ((inlineNum + inlineCount) <= maxInlineCount)) {
+              val notTooDeep = !outerExpr.isDefined || ((inlineNum + inlineCount) <= maxInlineCount)
+              if (canInline(refExpr, outerExpr) && notTooDeep) {
                 inlineCount += inlineNum
                 refExpr
               } else {
diff --git a/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala b/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
index 1bf7261f..b074e712 100644
--- a/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
+++ b/src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala
@@ -241,4 +241,15 @@ class InlineBooleanExpressionsSpec extends FirrtlFlatSpec {
         |    out <= _f""".stripMargin
     firrtlEquivalenceTest(input, Seq(new InlineBooleanExpressions))
   }
+
+  it should "avoid inlining when it would create context-sensitivity bugs" in {
+    val input =
+      """circuit AddNot:
+        |  module AddNot:
+        |    input a: UInt<1>
+        |    input b: UInt<1>
+        |    output o: UInt<2>
+        |    o <= add(a, not(b))""".stripMargin
+    firrtlEquivalenceTest(input, Seq(new InlineBooleanExpressions))
+  }
 }