From c8f22cf195eb2e096d95b298c69142b303a7c7a0 Mon Sep 17 00:00:00 2001
From: Albert Magyar
Date: Fri, 28 Aug 2020 15:01:56 -0700
Subject: Restrict boolean inlining to avoid context-sensitive width bugs

* Restore depth-agnostic inlining for simple 'lhs = ref' bool assignments
* Address review comments
* Run scalafmt
---
 .../transforms/InlineBooleanExpressions.scala      | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

(limited to 'src')

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 {
-- 
cgit v1.2.3


From 8fa7b99d3dff5f199455fa67e90a7dfa6941b8ff Mon Sep 17 00:00:00 2001
From: Albert Magyar
Date: Fri, 28 Aug 2020 15:46:34 -0700
Subject: Add test for InlineBooleanExpressions add-not example

---
 src/test/scala/firrtlTests/InlineBooleanExpressionsSpec.scala | 11 +++++++++++
 1 file changed, 11 insertions(+)

(limited to 'src')

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))
+  }
 }
-- 
cgit v1.2.3