From 94aeeb1a5c2fe38777a9004ba36f8b353e96b292 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 8 Jul 2022 23:44:45 +0000 Subject: CompileOptions: add and use emitStrictConnects (#2622) (#2623) (cherry picked from commit 11e8cc60d6268301cff352b8a1d7c4d672b5be11) Co-authored-by: Megan Wachs --- core/src/main/scala/chisel3/CompileOptions.scala | 35 +++++++---- core/src/main/scala/chisel3/Data.scala | 6 ++ .../main/scala/chisel3/internal/BiConnect.scala | 70 +++++++++++++--------- .../CompatibilityInteroperabilitySpec.scala | 39 ++++++++++++ 4 files changed, 113 insertions(+), 37 deletions(-) diff --git a/core/src/main/scala/chisel3/CompileOptions.scala b/core/src/main/scala/chisel3/CompileOptions.scala index 2764b652..d7d30306 100644 --- a/core/src/main/scala/chisel3/CompileOptions.scala +++ b/core/src/main/scala/chisel3/CompileOptions.scala @@ -6,25 +6,37 @@ import scala.language.experimental.macros import scala.reflect.macros.blackbox.Context trait CompileOptions { - // Should Record connections require a strict match of fields. - // If true and the same fields aren't present in both source and sink, a MissingFieldException, - // MissingLeftFieldException, or MissingRightFieldException will be thrown. + + /** Should Record connections require a strict match of fields. + * + * If true and the same fields aren't present in both source and sink, a MissingFieldException, + * MissingLeftFieldException, or MissingRightFieldException will be thrown. + */ val connectFieldsMustMatch: Boolean - // When creating an object that takes a type argument, the argument must be unbound (a pure type). + + /** When creating an object that takes a type argument, the argument must be unbound (a pure type). */ val declaredTypeMustBeUnbound: Boolean - // If a connection operator fails, don't try the connection with the operands (source and sink) reversed. + + /** If a connection operator fails, don't try the connection with the operands (source and sink) reversed. */ val dontTryConnectionsSwapped: Boolean - // If connection directionality is not explicit, do not use heuristics to attempt to determine it. + + /** If connection directionality is not explicit, do not use heuristics to attempt to determine it. */ val dontAssumeDirectionality: Boolean - // Check that referenced Data have actually been declared. + + /** Check that referenced Data have actually been declared. */ val checkSynthesizable: Boolean - // Require explicit assignment of DontCare to generate "x is invalid" + + /** Require explicit assignment of DontCare to generate "x is invalid" */ val explicitInvalidate: Boolean - // Should the reset type of Module be a Bool or a Reset + + /** Should the reset type of Module be a Bool or a Reset */ val inferModuleReset: Boolean /** If marked true, then any Module which consumes `inferModuleReset=false` must also mix in [[RequireSyncReset]] */ def migrateInferModuleReset: Boolean = false + + /** Should connects emit as firrtl <= instead of <- */ + def emitStrictConnects: Boolean = true } object CompileOptions { @@ -39,6 +51,7 @@ object CompileOptions { } object ExplicitCompileOptions { + case class CompileOptionsClass( // Should Record connections require a strict match of fields. // If true and the same fields aren't present in both source and sink, a MissingFieldException, @@ -68,7 +81,9 @@ object ExplicitCompileOptions { checkSynthesizable = false, explicitInvalidate = false, inferModuleReset = false - ) + ) { + override def emitStrictConnects = false + } // Collection of "strict" connection compile options, preferred for new code. implicit val Strict = new CompileOptionsClass( diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index cb8f4683..592ebe25 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -596,6 +596,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { private[chisel3] def badConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = throwException(s"cannot connect ${this} and ${that}") + private[chisel3] def connect( that: Data )( @@ -609,6 +610,9 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { case _: ReadOnlyBinding => throwException(s"Cannot reassign to read-only $this") case _ => // fine } + } + if (connectCompileOptions.emitStrictConnects) { + try { MonoConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.referenceUserModule) } catch { @@ -636,6 +640,8 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { case (_: DontCareBinding, _) => throw BiConnect.DontCareCantBeSink case _ => // fine } + } + if (connectCompileOptions.emitStrictConnects) { try { BiConnect.connect(sourceInfo, connectCompileOptions, this, that, Builder.referenceUserModule) } catch { diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala index 2d6c9e4a..e8fb2361 100644 --- a/core/src/main/scala/chisel3/internal/BiConnect.scala +++ b/core/src/main/scala/chisel3/internal/BiConnect.scala @@ -55,6 +55,24 @@ private[chisel3] object BiConnect { * There is some cleverness in the use of internal try-catch to catch exceptions * during the recursive decent and then rethrow them with extra information added. * This gives the user a 'path' to where in the connections things went wrong. + * + * == Chisel Semantics and how they emit to firrtl == + * + * 1. Strict Bi-Connect (all fields as seen by firrtl must match exactly) + * `a <= b` + * + * 2. Strict Bi-Connect (implemented as being field-blasted because we know all firrtl fields would not match exactly) + * `a.foo <= b.foo, b.bar <= a.bar` + * + * 3. Not-Strict Bi-Connect (firrtl will allow fields to not match exactly) + * `a <- b` + * + * 4. Mixed Semantic Bi-Connect (some fields need to be handled differently) + * `a.foo <= b.foo` (case 2), `b.bar <- a.bar` (case 3) + * + * - The decision on 1 vs 2 is based on structural type -- if same type once emitted to firrtl, emit 1, otherwise emit 2 + * - 1/2 vs 3 is based on CompileOptions at connection point e.g. at `<>` , emit 3 if `emitStrictConnects = false` for either side + * - 4 is a special case of 2 turning into 3 for some subfields, when either side's subfield at `extends Bundle/Record` has `emitStrictConnects = false` */ def connect( sourceInfo: SourceInfo, @@ -140,8 +158,8 @@ private[chisel3] object BiConnect { // 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) + val emitStrictConnects: Boolean = + left_r.compileOptions.emitStrictConnects && right_r.compileOptions.emitStrictConnects // chisel3 <> is commutative but FIRRTL <- is not val flipConnection = @@ -161,40 +179,38 @@ private[chisel3] object BiConnect { ) ) { pushCommand(Connect(sourceInfo, leftReified.get.lref, rightReified.get.lref)) - } else if (notStrict) { - newLeft.bulkConnect(newRight)(sourceInfo, ExplicitCompileOptions.NotStrict) + } else if (!emitStrictConnects) { + newLeft.legacyConnect(newRight)(sourceInfo) } else { recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod) } - // Handle Records connected to DontCare (change to NotStrict) + // Handle Records connected to DontCare case (left_r: Record, DontCare) => - left_r.compileOptions match { - case ExplicitCompileOptions.NotStrict => - left.bulkConnect(right)(sourceInfo, ExplicitCompileOptions.NotStrict) - case _ => - // For each field in left, descend with right - for ((field, left_sub) <- left_r.elements) { - try { - connect(sourceInfo, connectCompileOptions, left_sub, right, context_mod) - } catch { - case BiConnectException(message) => throw BiConnectException(s".$field$message") - } + if (!left_r.compileOptions.emitStrictConnects) { + left.legacyConnect(right)(sourceInfo) + } else { + // For each field in left, descend with right + for ((field, left_sub) <- left_r.elements) { + try { + connect(sourceInfo, connectCompileOptions, left_sub, right, context_mod) + } catch { + case BiConnectException(message) => throw BiConnectException(s".$field$message") } + } } case (DontCare, right_r: Record) => - right_r.compileOptions match { - case ExplicitCompileOptions.NotStrict => - left.bulkConnect(right)(sourceInfo, ExplicitCompileOptions.NotStrict) - case _ => - // For each field in left, descend with right - for ((field, right_sub) <- right_r.elements) { - try { - connect(sourceInfo, connectCompileOptions, left, right_sub, context_mod) - } catch { - case BiConnectException(message) => throw BiConnectException(s".$field$message") - } + if (!right_r.compileOptions.emitStrictConnects) { + left.legacyConnect(right)(sourceInfo) + } else { + // For each field in left, descend with right + for ((field, right_sub) <- right_r.elements) { + try { + connect(sourceInfo, connectCompileOptions, left, right_sub, context_mod) + } catch { + case BiConnectException(message) => throw BiConnectException(s".$field$message") } + } } // Left and right are different subtypes of Data so fail diff --git a/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala b/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala index 70dcda48..1e199297 100644 --- a/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala +++ b/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala @@ -351,4 +351,43 @@ class CompatibilityInteroperabilitySpec extends ChiselFlatSpec { compile(new Top(true)) compile(new Top(false)) } + + "A unidirectional but flipped Bundle with something close to NotStrict compileOptions, but not exactly" should "bulk connect in import chisel3._ code correctly" in { + object Compat { + import Chisel.{defaultCompileOptions => _, _} + // arbitrary thing to make this *not* exactly NotStrict + implicit val defaultCompileOptions = new chisel3.ExplicitCompileOptions.CompileOptionsClass( + connectFieldsMustMatch = false, + declaredTypeMustBeUnbound = false, + dontTryConnectionsSwapped = false, + dontAssumeDirectionality = false, + checkSynthesizable = false, + explicitInvalidate = false, + inferModuleReset = true // different from NotStrict, to ensure case class equivalence to NotStrict is false + ) { + override def emitStrictConnects = false + } + + 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) + }) + } + } + import chisel3._ + import Compat.{defaultCompileOptions => _, _} + 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)) + } } -- cgit v1.2.3