Emit FIRRTL bulkconnects whenever possible (#2381) (#2440)

Chisel <> semantics differ somewhat from FIRRTL <= semantics,
so we only emit <= when it would be legal. Otherwise we continue
the old behavior of emitting a connection for every leaf-level
Element.

Co-authored-by: Deborah Soung <debs@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
(cherry picked from commit 3553a1583403824718923a6cc530cec3b38f5704)

Co-authored-by: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>

3 files changed, 298 insertions, 48 deletions

diff --git a/core/src/main/scala/chisel3/experimental/dataview/package.scala b/core/src/main/scala/chisel3/experimental/dataview/package.scala
index 3278d82c..891ecb81 100644
--- a/core/src/main/scala/chisel3/experimental/dataview/package.scala
+++ b/core/src/main/scala/chisel3/experimental/dataview/package.scala
@@ -262,4 +262,14 @@ package object dataview {
     }
   }
 
+  /** Determine the target of a View if it is a single Target
+    *
+    * @note An Aggregate may be a view of unrelated [[Data]] (eg. like a Seq or tuple) and thus this
+    *       there is no single Data representing the Target and this function will return None
+    * @return The single Data target of this view or None if a single Data doesn't exist
+    */
+  private[chisel3] def reifyToAggregate(data: Data): Option[Aggregate] = reifySingleData(data) match {
+    case Some(a: Aggregate) => Some(a)
+    case other => None
+  }
 }
diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala
index 5b4ad1b9..a8b425f5 100644
--- a/core/src/main/scala/chisel3/internal/BiConnect.scala
+++ b/core/src/main/scala/chisel3/internal/BiConnect.scala
@@ -3,15 +3,14 @@
 package chisel3.internal
 
 import chisel3._
-import chisel3.experimental.dataview.reify
+import chisel3.experimental.dataview.{isView, reify, reifyToAggregate}
 import chisel3.experimental.{attach, Analog, BaseModule}
 import chisel3.internal.Builder.pushCommand
-import chisel3.internal.firrtl.{Connect, DefInvalid}
+import chisel3.internal.firrtl.{Connect, Converter, DefInvalid}
 
 import scala.language.experimental.macros
 import chisel3.internal.sourceinfo._
-
-import scala.annotation.tailrec
+import _root_.firrtl.passes.CheckTypes
 
 /**
   * BiConnect.connect executes a bidirectional connection element-wise.
@@ -91,12 +90,28 @@ private[chisel3] object BiConnect {
         if (left_v.length != right_v.length) {
           throw MismatchedVecException
         }
-        for (idx <- 0 until left_v.length) {
-          try {
-            implicit val compileOptions = connectCompileOptions
-            connect(sourceInfo, connectCompileOptions, left_v(idx), right_v(idx), context_mod)
-          } catch {
-            case BiConnectException(message) => throw BiConnectException(s"($idx)$message")
+
+        val leftReified:  Option[Aggregate] = if (isView(left_v)) reifyToAggregate(left_v) else Some(left_v)
+        val rightReified: Option[Aggregate] = if (isView(right_v)) reifyToAggregate(right_v) else Some(right_v)
+
+        if (
+          leftReified.nonEmpty && rightReified.nonEmpty && canBulkConnectAggregates(
+            leftReified.get,
+            rightReified.get,
+            sourceInfo,
+            connectCompileOptions,
+            context_mod
+          )
+        ) {
+          pushCommand(Connect(sourceInfo, leftReified.get.lref, rightReified.get.lref))
+        } else {
+          for (idx <- 0 until left_v.length) {
+            try {
+              implicit val compileOptions = connectCompileOptions
+              connect(sourceInfo, connectCompileOptions, left_v(idx), right_v(idx), context_mod)
+            } catch {
+              case BiConnectException(message) => throw BiConnectException(s"($idx)$message")
+            }
           }
         }
       }
@@ -122,29 +137,31 @@ private[chisel3] object BiConnect {
           }
         }
       }
-      // Handle Records defined in Chisel._ code by emitting a FIRRTL partial connect
+      // Handle Records defined in Chisel._ code by emitting a FIRRTL bulk
+      // connect when possible and a partial connect otherwise
       case pair @ (left_r: Record, right_r: Record) =>
         val notStrict =
           Seq(left_r.compileOptions, right_r.compileOptions).contains(ExplicitCompileOptions.NotStrict)
-        if (notStrict) {
-          // 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
-            }
-          }
-          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
+
+        // chisel3 <> is commutative but FIRRTL <- is not
+        val flipConnection =
+          !MonoConnect.canBeSink(left_r, context_mod) || !MonoConnect.canBeSource(right_r, context_mod)
+        val (newLeft, newRight) = if (flipConnection) (right_r, left_r) else (left_r, right_r)
+
+        val leftReified:  Option[Aggregate] = if (isView(newLeft)) reifyToAggregate(newLeft) else Some(newLeft)
+        val rightReified: Option[Aggregate] = if (isView(newRight)) reifyToAggregate(newRight) else Some(newRight)
+
+        if (
+          leftReified.nonEmpty && rightReified.nonEmpty && canBulkConnectAggregates(
+            leftReified.get,
+            rightReified.get,
+            sourceInfo,
+            connectCompileOptions,
+            context_mod
+          )
+        ) {
+          pushCommand(Connect(sourceInfo, leftReified.get.lref, rightReified.get.lref))
+        } else if (notStrict) {
           newLeft.bulkConnect(newRight)(sourceInfo, ExplicitCompileOptions.NotStrict)
         } else {
           recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod)
@@ -218,6 +235,59 @@ private[chisel3] object BiConnect {
     }
   }
 
+  /** Check whether two aggregates can be bulk connected (<=) in FIRRTL. From the
+    * FIRRTL specification, the following must hold for bulk connection:
+    *
+    *   1. The types of the left-hand and right-hand side expressions must be
+    *       equivalent.
+    *   2. The bit widths of the two expressions must allow for data to always
+    *        flow from a smaller bit width to an equal size or larger bit width.
+    *   3. The flow of the left-hand side expression must be sink or duplex
+    *   4. Either the flow of the right-hand side expression is source or duplex,
+    *      or the right-hand side expression has a passive type.
+    */
+  private[chisel3] def canBulkConnectAggregates(
+    sink:                  Aggregate,
+    source:                Aggregate,
+    sourceInfo:            SourceInfo,
+    connectCompileOptions: CompileOptions,
+    context_mod:           RawModule
+  ): Boolean = {
+
+    // check that the aggregates have the same types
+    def typeCheck = CheckTypes.validConnect(
+      Converter.extractType(sink, sourceInfo),
+      Converter.extractType(source, sourceInfo)
+    )
+
+    // check records live in appropriate contexts
+    def contextCheck =
+      MonoConnect.aggregateConnectContextCheck(
+        sourceInfo,
+        connectCompileOptions,
+        sink,
+        source,
+        context_mod
+      )
+
+    // sink must be writable and must also not be a literal
+    def bindingCheck = sink.topBinding match {
+      case _: ReadOnlyBinding => false
+      case _ => true
+    }
+
+    // check data can flow between provided aggregates
+    def flow_check = MonoConnect.canBeSink(sink, context_mod) && MonoConnect.canBeSource(source, context_mod)
+
+    // do not bulk connect source literals (results in infinite recursion from calling .ref)
+    def sourceNotLiteralCheck = source.topBinding match {
+      case _: LitBinding => false
+      case _ => true
+    }
+
+    typeCheck && contextCheck && bindingCheck && flow_check && sourceNotLiteralCheck
+  }
+
   // These functions (finally) issue the connection operation
   // Issue with right as sink, left as source
   private def issueConnectL2R(left: Element, right: Element)(implicit sourceInfo: SourceInfo): Unit = {
diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala
index b4d9aeff..40056c89 100644
--- a/core/src/main/scala/chisel3/internal/MonoConnect.scala
+++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala
@@ -5,11 +5,13 @@ package chisel3.internal
 import chisel3._
 import chisel3.experimental.{Analog, BaseModule, EnumType, FixedPoint, Interval, UnsafeEnum}
 import chisel3.internal.Builder.pushCommand
-import chisel3.experimental.dataview.reify
-import chisel3.internal.firrtl.{Connect, DefInvalid}
+import chisel3.internal.firrtl.{Connect, Converter, DefInvalid}
+import chisel3.experimental.dataview.{isView, reify, reifyToAggregate}
 
 import scala.language.experimental.macros
 import chisel3.internal.sourceinfo.SourceInfo
+import _root_.firrtl.passes.CheckTypes
+import scala.annotation.tailrec
 
 /**
   * MonoConnect.connect executes a mono-directional connection element-wise.
@@ -129,12 +131,28 @@ private[chisel3] object MonoConnect {
       // Handle Vec case
       case (sink_v: Vec[Data @unchecked], source_v: Vec[Data @unchecked]) =>
         if (sink_v.length != source_v.length) { throw MismatchedVecException }
-        for (idx <- 0 until sink_v.length) {
-          try {
-            implicit val compileOptions = connectCompileOptions
-            connect(sourceInfo, connectCompileOptions, sink_v(idx), source_v(idx), context_mod)
-          } catch {
-            case MonoConnectException(message) => throw MonoConnectException(s"($idx)$message")
+
+        val sinkReified:   Option[Aggregate] = if (isView(sink_v)) reifyToAggregate(sink_v) else Some(sink_v)
+        val sourceReified: Option[Aggregate] = if (isView(source_v)) reifyToAggregate(source_v) else Some(source_v)
+
+        if (
+          sinkReified.nonEmpty && sourceReified.nonEmpty && canBulkConnectAggregates(
+            sinkReified.get,
+            sourceReified.get,
+            sourceInfo,
+            connectCompileOptions,
+            context_mod
+          )
+        ) {
+          pushCommand(Connect(sourceInfo, sinkReified.get.lref, sourceReified.get.ref))
+        } else {
+          for (idx <- 0 until sink_v.length) {
+            try {
+              implicit val compileOptions = connectCompileOptions
+              connect(sourceInfo, connectCompileOptions, sink_v(idx), source_v(idx), context_mod)
+            } catch {
+              case MonoConnectException(message) => throw MonoConnectException(s"($idx)$message")
+            }
           }
         }
       // Handle Vec connected to DontCare. Apply the DontCare to individual elements.
@@ -150,19 +168,34 @@ private[chisel3] object MonoConnect {
 
       // Handle Record case
       case (sink_r: Record, source_r: Record) =>
-        // For each field, descend with right
-        for ((field, sink_sub) <- sink_r.elements) {
-          try {
-            source_r.elements.get(field) match {
-              case Some(source_sub) => connect(sourceInfo, connectCompileOptions, sink_sub, source_sub, context_mod)
-              case None => {
-                if (connectCompileOptions.connectFieldsMustMatch) {
-                  throw MissingFieldException(field)
+        val sinkReified:   Option[Aggregate] = if (isView(sink_r)) reifyToAggregate(sink_r) else Some(sink_r)
+        val sourceReified: Option[Aggregate] = if (isView(source_r)) reifyToAggregate(source_r) else Some(source_r)
+
+        if (
+          sinkReified.nonEmpty && sourceReified.nonEmpty && canBulkConnectAggregates(
+            sinkReified.get,
+            sourceReified.get,
+            sourceInfo,
+            connectCompileOptions,
+            context_mod
+          )
+        ) {
+          pushCommand(Connect(sourceInfo, sinkReified.get.lref, sourceReified.get.ref))
+        } else {
+          // For each field, descend with right
+          for ((field, sink_sub) <- sink_r.elements) {
+            try {
+              source_r.elements.get(field) match {
+                case Some(source_sub) => connect(sourceInfo, connectCompileOptions, sink_sub, source_sub, context_mod)
+                case None => {
+                  if (connectCompileOptions.connectFieldsMustMatch) {
+                    throw MissingFieldException(field)
+                  }
                 }
               }
+            } catch {
+              case MonoConnectException(message) => throw MonoConnectException(s".$field$message")
             }
-          } catch {
-            case MonoConnectException(message) => throw MonoConnectException(s".$field$message")
           }
         }
       // Handle Record connected to DontCare. Apply the DontCare to individual elements.
@@ -190,6 +223,143 @@ private[chisel3] object MonoConnect {
       case (sink, source) => throw MismatchedException(sink, source)
     }
 
+  /** Determine if a valid connection can be made between a source [[Aggregate]] and sink
+    * [[Aggregate]] given their parent module and directionality context
+    *
+    * @return whether the source and sink exist in an appropriate context to be connected
+    */
+  private[chisel3] def aggregateConnectContextCheck(
+    implicit sourceInfo:   SourceInfo,
+    connectCompileOptions: CompileOptions,
+    sink:                  Aggregate,
+    source:                Aggregate,
+    context_mod:           RawModule
+  ): Boolean = {
+    import ActualDirection.{Bidirectional, Input, Output}
+    // If source has no location, assume in context module
+    // This can occur if is a literal, unbound will error previously
+    val sink_mod:   BaseModule = sink.topBinding.location.getOrElse(throw UnwritableSinkException(sink, source))
+    val source_mod: BaseModule = source.topBinding.location.getOrElse(context_mod)
+
+    val sink_parent = Builder.retrieveParent(sink_mod, context_mod).getOrElse(None)
+    val source_parent = Builder.retrieveParent(source_mod, context_mod).getOrElse(None)
+
+    val sink_is_port = sink.topBinding match {
+      case PortBinding(_) => true
+      case _              => false
+    }
+    val source_is_port = source.topBinding match {
+      case PortBinding(_) => true
+      case _              => false
+    }
+
+    if (!checkWhenVisibility(sink)) {
+      throw SinkEscapedWhenScopeException(sink)
+    }
+
+    if (!checkWhenVisibility(source)) {
+      throw SourceEscapedWhenScopeException(source)
+    }
+
+    // CASE: Context is same module that both sink node and source node are in
+    if ((context_mod == sink_mod) && (context_mod == source_mod)) {
+      sink.direction != Input
+    }
+
+    // CASE: Context is same module as sink node and source node is in a child module
+    else if ((sink_mod == context_mod) && (source_parent == context_mod)) {
+      // NOTE: Workaround for bulk connecting non-agnostified FIRRTL ports
+      // See: https://github.com/freechipsproject/firrtl/issues/1703
+      // Original behavior should just check if the sink direction is an Input
+      val sinkCanBeInput = sink.direction match {
+        case Input            => true
+        case Bidirectional(_) => true
+        case _                => false
+      }
+      // Thus, right node better be a port node and thus have a direction
+      if (!source_is_port) { !connectCompileOptions.dontAssumeDirectionality }
+      else if (sinkCanBeInput) {
+        if (source.direction == Output) {
+          !connectCompileOptions.dontTryConnectionsSwapped
+        } else { false }
+      } else { true }
+    }
+
+    // CASE: Context is same module as source node and sink node is in child module
+    else if ((source_mod == context_mod) && (sink_parent == context_mod)) {
+      // NOTE: Workaround for bulk connecting non-agnostified FIRRTL ports
+      // See: https://github.com/freechipsproject/firrtl/issues/1703
+      // Original behavior should just check if the sink direction is an Input
+      sink.direction match {
+        case Input            => true
+        case Bidirectional(_) => true
+        case _                => false
+      }
+    }
+
+    // CASE: Context is the parent module of both the module containing sink node
+    //                                        and the module containing source node
+    //   Note: This includes case when sink and source in same module but in parent
+    else if ((sink_parent == context_mod) && (source_parent == context_mod)) {
+      // Thus both nodes must be ports and have a direction
+      if (!source_is_port) { !connectCompileOptions.dontAssumeDirectionality }
+      else if (sink_is_port) {
+        // NOTE: Workaround for bulk connecting non-agnostified FIRRTL ports
+        // See: https://github.com/freechipsproject/firrtl/issues/1703
+        // Original behavior should just check if the sink direction is an Input
+        sink.direction match {
+          case Input            => true
+          case Bidirectional(_) => true // NOTE: Workaround for non-agnostified ports
+          case _                => false
+        }
+      } else { false }
+    }
+
+    // Not quite sure where left and right are compared to current module
+    // so just error out
+    else false
+  }
+
+  /** Trace flow from child Data to its parent. */
+  @tailrec private[chisel3] def traceFlow(currentlyFlipped: Boolean, data: Data, context_mod: RawModule): 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, context_mod)
+      case PortBinding(enclosure) =>
+        val childPort = enclosure != context_mod
+        childPort ^ flipped ^ currentlyFlipped
+      case _ => true
+    }
+  }
+  def canBeSink(data:   Data, context_mod: RawModule): Boolean = traceFlow(true, data, context_mod)
+  def canBeSource(data: Data, context_mod: RawModule): Boolean = traceFlow(false, data, context_mod)
+
+  /** Check whether two aggregates can be bulk connected (<=) in FIRRTL. (MonoConnect case)
+    *
+    * Mono-directional bulk connects only work if all signals of the sink are unidirectional
+    * In the case of a sink aggregate with bidirectional signals, e.g. `Decoupled`,
+    * a `BiConnect` is necessary.
+    */
+  private[chisel3] def canBulkConnectAggregates(
+    sink:                  Aggregate,
+    source:                Aggregate,
+    sourceInfo:            SourceInfo,
+    connectCompileOptions: CompileOptions,
+    context_mod:           RawModule
+  ): Boolean = {
+    // Assuming we're using a <>, check if a bulk connect is valid in that case
+    def biConnectCheck =
+      BiConnect.canBulkConnectAggregates(sink, source, sourceInfo, connectCompileOptions, context_mod)
+
+    // Check that the Aggregate can be driven (not bidirectional or an input) to match Chisel semantics
+    def sinkCanBeDrivenCheck: Boolean =
+      sink.direction == ActualDirection.Output || sink.direction == ActualDirection.Unspecified
+
+    biConnectCheck && sinkCanBeDrivenCheck
+  }
+
   // This function (finally) issues the connection operation
   private def issueConnect(sink: Element, source: Element)(implicit sourceInfo: SourceInfo): Unit = {
     // If the source is a DontCare, generate a DefInvalid for the sink,