From ed1eb88d6ccdccd4b5802676cd8b69f5cc357e4f Mon Sep 17 00:00:00 2001
From: Jack Koenig
Date: Wed, 10 Mar 2021 20:14:03 -0800
Subject: Fix CSESubAccesses for SubAccesses with flips (#2112)

The flow of a LHS SubAccess node may still be SourceFlow if the type of
the Vec element has a flip. Tweak the logic of CSESubAccesses to check
every Expression flow while recursing instead of just the flow of the
final SubAccess.

Co-authored-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

Co-authored-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>---
 .../scala/firrtl/transforms/CSESubAccesses.scala   |  6 +++-
 .../transforms/CSESubAccessesSpec.scala            | 36 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

(limited to 'src')

diff --git a/src/main/scala/firrtl/transforms/CSESubAccesses.scala b/src/main/scala/firrtl/transforms/CSESubAccesses.scala
index 6ed3a5b5..6ca27a83 100644
--- a/src/main/scala/firrtl/transforms/CSESubAccesses.scala
+++ b/src/main/scala/firrtl/transforms/CSESubAccesses.scala
@@ -21,7 +21,9 @@ object CSESubAccesses {
     val acc = new mutable.ListBuffer[(SubAccess, Info)]
     def onExpr(outer: Statement)(expr: Expression): Unit = {
       // Need postorder because we want to visit inner SubAccesses first
-      expr.foreach(onExpr(outer))
+      // Stop recursing on any non-Source because flips can make the SubAccess a Source despite the
+      //   overall Expression being a Sink
+      if (flow(expr) == SourceFlow) expr.foreach(onExpr(outer))
       expr match {
         case e: SubAccess if flow(e) == SourceFlow => acc += e -> get_info(outer)
         case _ => // Do nothing
@@ -42,6 +44,8 @@ object CSESubAccesses {
 
   // Replaces all right-hand side SubAccesses with References
   private def replaceOnSourceExpr(replace: SubAccess => Reference)(expr: Expression): Expression = expr match {
+    // Stop is we ever see a non-SourceFlow
+    case e if flow(e) != SourceFlow => e
     // Don't traverse children of SubAccess, just replace it
     // Nested SubAccesses are handled during creation of the nodes that the references refer to
     case acc: SubAccess if flow(acc) == SourceFlow => replace(acc)
diff --git a/src/test/scala/firrtlTests/transforms/CSESubAccessesSpec.scala b/src/test/scala/firrtlTests/transforms/CSESubAccessesSpec.scala
index 55ce07df..f7d67026 100644
--- a/src/test/scala/firrtlTests/transforms/CSESubAccessesSpec.scala
+++ b/src/test/scala/firrtlTests/transforms/CSESubAccessesSpec.scala
@@ -184,4 +184,40 @@ class CSESubAccessesSpec extends FirrtlFlatSpec {
     compile(input) should be(parse(expected).serialize)
   }
 
+  it should "ignore flipped LHS SubAccesses" in {
+    val input = circuit(
+      s"""|input in : { foo : UInt<8> }
+          |input idx : UInt<1>
+          |input out : { flip foo : UInt<8> }[2]
+          |out[0].foo <= UInt(0)
+          |out[1].foo <= UInt(0)
+          |out[idx].foo <= in.foo"""
+    )
+    val expected = circuit(
+      s"""|input in : { foo : UInt<8> }
+          |input idx : UInt<1>
+          |input out : { flip foo : UInt<8> }[2]
+          |out[0].foo <= UInt(0)
+          |out[1].foo <= UInt(0)
+          |out[idx].foo <= in.foo"""
+    )
+    compile(input) should be(parse(expected).serialize)
+  }
+
+  it should "ignore SubAccesses of bidirectional aggregates" in {
+    val input = circuit(
+      s"""|input in : { flip foo : UInt<8>, bar : UInt<8> }
+          |input idx : UInt<2>
+          |output out : { flip foo : UInt<8>, bar : UInt<8> }[4]
+          |out[idx] <= in"""
+    )
+    val expected = circuit(
+      s"""|input in : { flip foo : UInt<8>, bar : UInt<8> }
+          |input idx : UInt<2>
+          |output out : { flip foo : UInt<8>, bar : UInt<8> }[4]
+          |out[idx] <= in"""
+    )
+    compile(input) should be(parse(expected).serialize)
+  }
+
 }
-- 
cgit v1.2.3