Fix chisel3 <> for compatibility Bundles (Take 3) (#2093)

Previous incomplete fixes in #2023 and #2031.

The legality of a FIRRTL connection is determined by type and flow.
Chisel does not have access to true flow information. Previous fix
attempts tried to use ActualDirection as a stand-in for flow, but it is
incorrect in many cases. This new approach checks the flows of the
lvalue and rvalues in the connect and flips the connection if either
the lvalue cannot be a sink or the rvalue cannot be a source.

2 files changed, 46 insertions, 8 deletions

diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala
index 4a9bb4f5..aa58cb95 100644
--- a/core/src/main/scala/chisel3/internal/BiConnect.scala
+++ b/core/src/main/scala/chisel3/internal/BiConnect.scala
@@ -11,6 +11,8 @@ import chisel3.internal.firrtl.{Connect, DefInvalid}
 import scala.language.experimental.macros
 import chisel3.internal.sourceinfo._
 
+import scala.annotation.tailrec
+
 /**
 * BiConnect.connect executes a bidirectional connection element-wise.
 *
@@ -120,15 +122,24 @@ private[chisel3] object BiConnect {
         val notStrict =
           Seq(left_r.compileOptions, right_r.compileOptions).contains(ExplicitCompileOptions.NotStrict)
         if (notStrict) {
-          // chisel3 <> is commutative but FIRRTL <- is not
-          val flipped = {
-            import ActualDirection._
-            // Everything is flipped when it's the port of a child
-            val childPort = left_r._parent.get != context_mod
-            val isFlipped = Seq(Bidirectional(Flipped), Input).contains(left_r.direction)
-            isFlipped ^ childPort
+          // Traces flow from a child Data to its parent
+          @tailrec def traceFlow(currentlyFlipped: Boolean, data: Data): Boolean = {
+            import SpecifiedDirection.{Input => SInput, Flip => SFlip}
+            val sdir = data.specifiedDirection
+            val flipped = sdir == SInput || sdir == SFlip
+            data.binding.get match {
+              case ChildBinding(parent) => traceFlow(flipped ^ currentlyFlipped, parent)
+              case PortBinding(enclosure) =>
+                val childPort = enclosure != context_mod
+                childPort ^ flipped ^ currentlyFlipped
+              case _ => true
+            }
           }
-          val (newLeft, newRight) = if (flipped) pair.swap else pair
+          def canBeSink(data: Data): Boolean = traceFlow(true, data)
+          def canBeSource(data: Data): Boolean = traceFlow(false, data)
+          // chisel3 <> is commutative but FIRRTL <- is not
+          val flipConnection = !canBeSink(left_r) || !canBeSource(right_r)
+          val (newLeft, newRight) = if (flipConnection) pair.swap else pair
           newLeft.bulkConnect(newRight)(sourceInfo, ExplicitCompileOptions.NotStrict)
         } else {
           recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod)
diff --git a/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala b/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala
index 28b8bc80..1795cc1f 100644
--- a/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala
+++ b/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala
@@ -332,5 +332,32 @@ class CompatibiltyInteroperabilitySpec extends ChiselFlatSpec {
       }
     }
   }
+
+  "A unidirectional but flipped Bundle" should "bulk connect in import chisel3._ code correctly" in {
+    object Compat {
+      import Chisel._
+      class MyBundle(extraFlip: Boolean) extends Bundle {
+        private def maybeFlip[T <: Data](t: T): T = if (extraFlip) t.flip else t
+        val foo = maybeFlip(new Bundle {
+          val bar = UInt(INPUT, width = 8)
+        })
+        override def cloneType = (new MyBundle(extraFlip)).asInstanceOf[this.type]
+      }
+    }
+    import chisel3._
+    import Compat._
+    class Top(extraFlip: Boolean) extends RawModule {
+      val port = IO(new MyBundle(extraFlip))
+      val wire = Wire(new MyBundle(extraFlip))
+      port <> DontCare
+      wire <> DontCare
+      port <> wire
+      wire <> port
+      port.foo <> wire.foo
+      wire.foo <> port.foo
+    }
+    compile(new Top(true))
+    compile(new Top(false))
+  }
 }