Emit FIRRTL bulkconnects whenever possible

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, reifySingleData}
 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,19 @@ 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")
+        if (canBulkConnectAggregates(left_v, right_v, sourceInfo, connectCompileOptions, context_mod)) {
+          val leftReified:  Data = if (isView(left_v)) reifySingleData(left_v).get else left_v
+          val rightReified: Data = if (isView(right_v)) reifySingleData(right_v).get else right_v
+
+          pushCommand(Connect(sourceInfo, leftReified.lref, rightReified.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 +128,24 @@ 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)
+
+        // Check whether Records can be bulk connected (all elements can be connected)
+        if (canBulkConnectAggregates(newLeft, newRight, sourceInfo, connectCompileOptions, context_mod)) {
+          val leftReified:  Data = if (isView(newLeft)) reifySingleData(newLeft).get else newLeft
+          val rightReified: Data = if (isView(newRight)) reifySingleData(newRight).get else newRight
+
+          pushCommand(Connect(sourceInfo, leftReified.lref, rightReified.lref))
+        } else if (notStrict) {
           newLeft.bulkConnect(newRight)(sourceInfo, ExplicitCompileOptions.NotStrict)
         } else {
           recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod)
@@ -218,6 +219,91 @@ 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
+    val typeCheck = CheckTypes.validConnect(
+      Converter.extractType(sink, sourceInfo),
+      Converter.extractType(source, sourceInfo)
+    )
+    // TODO do we need elementwise check for bundles?
+//    val elemsMatch = if(sink.elements.size == source.elements.size) {
+//      val elemValidConnect = sink.elements.zip(source.elements).map {
+//        case (sink, source) => CheckTypes.validConnect(
+//          Converter.extractType(sink._2, sourceInfo),
+//          Converter.extractType(source._2, sourceInfo))
+//        case _ => false
+//      }
+//      elemValidConnect.foldLeft(true)(_ && _)
+//    } else false
+
+    // do not bulk connect DataViews if they don't both reify to the same type
+    // or if their types do not exist
+    val sinkIsView = isView(sink)
+    val sourceIsView = isView(source)
+    var sourceReified: Aggregate = source
+    var sinkReified:   Aggregate = sink
+    if (sinkIsView || sourceIsView) {
+      val sourceType =
+        if (sourceIsView) reifySingleData(source) match {
+          case Some(data: Aggregate) =>
+            sourceReified = data
+            Converter.extractType(data, sourceInfo)
+          case None => None
+        }
+        else Converter.extractType(source, sourceInfo)
+      val sinkType =
+        if (sinkIsView) reifySingleData(sink) match {
+          case Some(data: Aggregate) =>
+            sinkReified = data
+            Converter.extractType(data, sourceInfo)
+          case None => None
+        }
+        else Converter.extractType(sink, sourceInfo)
+
+      if (sourceType != sinkType || sourceType == None)
+        return false
+    }
+
+    // check records live in appropriate contexts
+    val contextCheck =
+      MonoConnect.aggregateConnectContextCheck(
+        sourceInfo,
+        connectCompileOptions,
+        sinkReified,
+        sourceReified,
+        context_mod
+      )
+
+    // sink must be writable
+    val bindingCheck = sink.topBinding match {
+      case _: ReadOnlyBinding => false
+      case _ => true
+    }
+
+    // check data can flow between provided aggregates
+    val flow_check = MonoConnect.canBeSink(sink, context_mod) && MonoConnect.canBeSource(source, context_mod)
+
+    typeCheck && contextCheck && bindingCheck && flow_check
+  }
+
   // 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 = {

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, reifySingleData}
 
 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,19 @@ 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")
+        if (canBulkConnectAggregates(sink_v, source_v, sourceInfo, connectCompileOptions, context_mod)) {
+          val sinkReified:   Data = if (isView(sink_v)) reifySingleData(sink_v).get else sink_v
+          val sourceReified: Data = if (isView(source_v)) reifySingleData(source_v).get else source_v
+
+          pushCommand(Connect(sourceInfo, sinkReified.lref, sourceReified.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 +159,26 @@ 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)
+        if (canBulkConnectAggregates(sink_r, source_r, sourceInfo, connectCompileOptions, context_mod)) {
+          val sinkReified:   Data = if (isView(sink_r)) reifySingleData(sink_r).get else sink_r
+          val sourceReified: Data = if (isView(source_r)) reifySingleData(source_r).get else source_r
+
+          pushCommand(Connect(sourceInfo, sinkReified.lref, sourceReified.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 +206,138 @@ private[chisel3] object MonoConnect {
       case (sink, source) => throw MismatchedException(sink, source)
     }
 
+  /** Check [[Aggregate]] visibility. */
+  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
+    val biConnectCheck =
+      BiConnect.canBulkConnectAggregates(sink, source, sourceInfo, connectCompileOptions, context_mod)
+
+    // Check that the Aggregate's child signals are all strictly inputs (not bidirectional)
+    val sinkIsInputCheck: Boolean = sink.direction == ActualDirection.Output
+
+    biConnectCheck && sinkIsInputCheck
+  }
+
   // 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,