From c313e137d4e562ef20195312501840ceab8cbc6a Mon Sep 17 00:00:00 2001 From: Adam Izraelevitz Date: Thu, 26 Oct 2017 12:39:42 -0700 Subject: Invalidateapi (#645) * Require explicit connection to DontCare to generate "is invalid". * Add tests for RefNotInitializedException. Currently, we fail the when ... otherwise ... * Disable ScalaTest shrinking on error in ComplexAssignSpec. * fix broken merge; still some binding issues * cleanup DontCare connection checks; add missing directions to test module IOs * Have library code inherit compileOptions from the enclosing Module (if it exists). * work around current firrtl uninitialized references with Strict compile options and explicitInvalidate * more CompileOptions cleanup; move test-specific defines to package object * minimize differences with master * set default CompileOptions.explicitInvalidate to false until we fix the FIRRTL when issue * ignore the StrictCompiler property checks (until CompileOptions.explicitInvalidate is defaulted to true) * Revert "more CompileOptions cleanup; move test-specific defines to package object" This reverts commit e4486edcba990d150e76e08a2fc6abca033556e0. * Revert "work around current firrtl uninitialized references with Strict compile options and explicitInvalidate" This reverts commit 426faa430a62c3dac2dbdf33044d3386d4243157. * remove unused code * Convert to binding-based DontCare implementation * comment cleanup to minimize differences with master * Tentatively remove possibly redundant DefInvalid on module ports. * Respond to code review change request. - backout build.sbt change - correct indentation - handle bulk of DontCare semantics in elemConnect() - have DontCare extend Element, not Data (eliminate most Object specific methods - add comments indicating reason for explicit DontCare connections * Initialize test elements without requiring a DontCare. * Respond to review change requests. - DontCare should work on left or right side in BiDirectional connections - call bind() to set DontCare binding instead of messing with internal variables - DontCares are only equivalent with DontCares - clean up processWhens() definition * Eliminate DontCare connection to inputs in MonoConnect(). * Pull aggregates apart for the purpose of DontCare connections. * Restore the explicit (conditionally executed) ports DefInvalidin ImplicitModule() * Don't add DontCare's to the module list of _ids. * Add missing DefInvalid() to LegacyModule(). * Respond to review requests: add DontCare BiConnect Vec, remove null parent hack to avoid addId(), initialize singletons early in Builder * Move DontCare out of chisel3.experimental. --- .../src/main/scala/chisel3/core/Aggregate.scala | 11 ++- .../src/main/scala/chisel3/core/BiConnect.scala | 87 +++++++++++++++++++--- .../src/main/scala/chisel3/core/Binding.scala | 3 + .../src/main/scala/chisel3/core/Bits.scala | 13 +++- .../main/scala/chisel3/core/CompileOptions.scala | 12 ++- .../src/main/scala/chisel3/core/Data.scala | 37 ++++++++- .../src/main/scala/chisel3/core/MonoConnect.scala | 35 ++++++++- .../src/main/scala/chisel3/core/UserModule.scala | 23 ++++-- .../src/main/scala/chisel3/internal/Builder.scala | 5 ++ 9 files changed, 196 insertions(+), 30 deletions(-) (limited to 'chiselFrontend/src/main/scala/chisel3') diff --git a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala index 47f4b3e7..88cc6ff6 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala @@ -67,8 +67,15 @@ sealed abstract class Aggregate extends Data { def getElements: Seq[Data] private[chisel3] def width: Width = getElements.map(_.width).foldLeft(0.W)(_ + _) - private[core] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = - pushCommand(BulkConnect(sourceInfo, this.lref, that.lref)) + private[core] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = { + // If the source is a DontCare, generate a DefInvalid for the sink, + // otherwise, issue a Connect. + if (that == DontCare) { + pushCommand(DefInvalid(sourceInfo, this.lref)) + } else { + pushCommand(BulkConnect(sourceInfo, this.lref, that.lref)) + } + } override def do_asUInt(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt = { SeqUtils.do_asUInt(flatten.map(_.asUInt())) diff --git a/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala b/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala index f2b2b7e1..d056efba 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala @@ -2,10 +2,8 @@ package chisel3.core -import chisel3.internal.Builder import chisel3.internal.Builder.pushCommand -import chisel3.internal.firrtl.{Attach, Connect} -import chisel3.internal.throwException +import chisel3.internal.firrtl.{Connect, DefInvalid} import scala.language.experimental.macros import chisel3.internal.sourceinfo._ @@ -46,6 +44,8 @@ object BiConnect { BiConnectException(s": Left ($left) and Right ($right) have different types.") def AttachAlreadyBulkConnectedException(sourceInfo: SourceInfo) = BiConnectException(sourceInfo.makeMessage(": Analog previously bulk connected at " + _)) + def DontCareCantBeSink = + BiConnectException(": DontCare cannot be a connection sink (LHS)") /** This function is what recursively tries to connect a left and right together @@ -54,7 +54,7 @@ object BiConnect { * 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. */ - def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Data, right: Data, context_mod: UserModule): Unit = + def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Data, right: Data, context_mod: UserModule): Unit = { (left, right) match { // Handle element case (root case) case (left_a: Analog, right_a: Analog) => @@ -69,9 +69,11 @@ object BiConnect { // TODO(twigg): Verify the element-level classes are connectable } // Handle Vec case - case (left_v: Vec[Data @unchecked], right_v: Vec[Data @unchecked]) => { - if(left_v.length != right_v.length) { throw MismatchedVecException } - for(idx <- 0 until left_v.length) { + case (left_v: Vec[Data@unchecked], right_v: Vec[Data@unchecked]) => { + 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) @@ -80,6 +82,28 @@ object BiConnect { } } } + // Handle Vec connected to DontCare + case (left_v: Vec[Data@unchecked], DontCare) => { + for (idx <- 0 until left_v.length) { + try { + implicit val compileOptions = connectCompileOptions + connect(sourceInfo, connectCompileOptions, left_v(idx), right, context_mod) + } catch { + case BiConnectException(message) => throw BiConnectException(s"($idx)$message") + } + } + } + // Handle DontCare connected to Vec + case (DontCare, right_v: Vec[Data@unchecked]) => { + for (idx <- 0 until right_v.length) { + try { + implicit val compileOptions = connectCompileOptions + connect(sourceInfo, connectCompileOptions, left, right_v(idx), context_mod) + } catch { + case BiConnectException(message) => throw BiConnectException(s"($idx)$message") + } + } + } // Handle Records defined in Chisel._ code (change to NotStrict) case (left_r: Record, right_r: Record) => (left_r.compileOptions, right_r.compileOptions) match { case (ExplicitCompileOptions.NotStrict, _) => @@ -89,9 +113,40 @@ object BiConnect { case _ => recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod) } + // Handle Records connected to DontCare (change to NotStrict) + 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") + } + } + } + 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") + } + } + } + // Left and right are different subtypes of Data so fail case (left, right) => throw MismatchedException(left.toString, right.toString) } + } // Do connection of two Records def recordConnect(sourceInfo: SourceInfo, @@ -128,11 +183,25 @@ object BiConnect { // 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 = { - pushCommand(Connect(sourceInfo, right.lref, left.ref)) + // Source and sink are ambiguous in the case of a Bi/Bulk Connect (<>). + // If either is a DontCareBinding, just issue a DefInvalid for the other, + // otherwise, issue a Connect. + (left.binding, right.binding) match { + case (lb: DontCareBinding, _) => pushCommand(DefInvalid(sourceInfo, right.lref)) + case (_, rb: DontCareBinding) => pushCommand(DefInvalid(sourceInfo, left.lref)) + case (_, _) => pushCommand(Connect(sourceInfo, right.lref, left.ref)) + } } // Issue with left as sink, right as source private def issueConnectR2L(left: Element, right: Element)(implicit sourceInfo: SourceInfo): Unit = { - pushCommand(Connect(sourceInfo, left.lref, right.ref)) + // Source and sink are ambiguous in the case of a Bi/Bulk Connect (<>). + // If either is a DontCareBinding, just issue a DefInvalid for the other, + // otherwise, issue a Connect. + (left.binding, right.binding) match { + case (lb: DontCareBinding, _) => pushCommand(DefInvalid(sourceInfo, right.lref)) + case (_, rb: DontCareBinding) => pushCommand(DefInvalid(sourceInfo, left.lref)) + case (_, _) => pushCommand(Connect(sourceInfo, left.lref, right.ref)) + } } // This function checks if element-level connection operation allowed. diff --git a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala index b09bab0e..3fdc383c 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala @@ -104,3 +104,6 @@ case class WireBinding(enclosure: UserModule) extends ConstrainedBinding case class ChildBinding(parent: Data) extends Binding { def location = parent.binding.location } +// A DontCare element has a specific Binding, somewhat like a literal. +// It is a source (RHS). It may only be connected/applied to sinks. +case class DontCareBinding() extends UnconstrainedBinding diff --git a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala index 090c320b..4df46b75 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala @@ -32,8 +32,15 @@ abstract class Element(private[chisel3] val width: Width) extends Data { def widthKnown: Boolean = width.known def name: String = getRef.name - private[core] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = - pushCommand(Connect(sourceInfo, this.lref, that.ref)) + private[core] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = { + // If the source is a DontCare, generate a DefInvalid for the sink, + // otherwise, issue a Connect. + if (that == DontCare) { + pushCommand(DefInvalid(sourceInfo, this.lref)) + } else { + pushCommand(Connect(sourceInfo, this.lref, that.ref)) + } + } } /** Exists to unify common interfaces of [[Bits]] and [[Reset]] @@ -799,7 +806,7 @@ sealed class FixedPoint private (width: Width, val binaryPoint: BinaryPoint, lit new FixedPoint(w, binaryPoint).asInstanceOf[this.type] override def connect (that: Data)(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions): Unit = that match { - case _: FixedPoint => super.connect(that) + case _: FixedPoint|DontCare => super.connect(that) case _ => this badConnect that } diff --git a/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala b/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala index 55e6d18c..e22519d9 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/CompileOptions.scala @@ -18,6 +18,8 @@ trait CompileOptions { val dontAssumeDirectionality: Boolean // Check that referenced Data have actually been declared. val checkSynthesizable: Boolean + // Require explicit assignment of DontCare to generate "x is invalid" + val explicitInvalidate: Boolean } object CompileOptions { @@ -44,7 +46,9 @@ object ExplicitCompileOptions { // 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. - val checkSynthesizable: Boolean + val checkSynthesizable: Boolean, + // Require an explicit DontCare assignment to generate a firrtl DefInvalid + val explicitInvalidate: Boolean ) extends CompileOptions // Collection of "not strict" connection compile options. @@ -55,7 +59,8 @@ object ExplicitCompileOptions { declaredTypeMustBeUnbound = false, dontTryConnectionsSwapped = false, dontAssumeDirectionality = false, - checkSynthesizable = false + checkSynthesizable = false, + explicitInvalidate = false ) // Collection of "strict" connection compile options, preferred for new code. @@ -65,6 +70,7 @@ object ExplicitCompileOptions { declaredTypeMustBeUnbound = true, dontTryConnectionsSwapped = true, dontAssumeDirectionality = true, - checkSynthesizable = true + checkSynthesizable = true, + explicitInvalidate = true ) } diff --git a/chiselFrontend/src/main/scala/chisel3/core/Data.scala b/chiselFrontend/src/main/scala/chisel3/core/Data.scala index aa286e0d..db99d6b3 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Data.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Data.scala @@ -8,6 +8,7 @@ import chisel3.internal._ import chisel3.internal.Builder.{pushCommand, pushOp} import chisel3.internal.firrtl._ import chisel3.internal.sourceinfo._ +import chisel3.core.BiConnect.DontCareCantBeSink /** User-specified directions. */ @@ -33,8 +34,8 @@ object SpecifiedDirection { case Input => Output } - /** Returns the effective UserDirection of this node given the parent's effective UserDirection - * and the user-specified UserDirection of this node. + /** Returns the effective SpecifiedDirection of this node given the parent's effective SpecifiedDirection + * and the user-specified SpecifiedDirection of this node. */ def fromParent(parentDirection: SpecifiedDirection, thisDirection: SpecifiedDirection) = (parentDirection, thisDirection) match { @@ -222,7 +223,7 @@ abstract class Data extends HasId { _specifiedDirection = direction } - /** This overwrites a relative UserDirection with an explicit one, and is used to implement + /** This overwrites a relative SpecifiedDirection with an explicit one, and is used to implement * the compatibility layer where, at the elements, Flip is Input and unspecified is Output. * DO NOT USE OUTSIDE THIS PURPOSE. THIS OPERATION IS DANGEROUS! */ @@ -311,6 +312,8 @@ abstract class Data extends HasId { requireIsHardware(that, s"data to be bulk-connected") (this.topBinding, that.topBinding) match { case (_: ReadOnlyBinding, _: ReadOnlyBinding) => throwException(s"Both $this and $that are read-only") + // DontCare cannot be a sink (LHS) + case (_: DontCareBinding, _) => throw DontCareCantBeSink case _ => // fine } try { @@ -425,7 +428,9 @@ trait WireFactory { x.bind(WireBinding(Builder.forcedUserModule)) pushCommand(DefWire(sourceInfo, x)) - pushCommand(DefInvalid(sourceInfo, x.ref)) + if (!compileOptions.explicitInvalidate) { + pushCommand(DefInvalid(sourceInfo, x.ref)) + } x } @@ -454,3 +459,27 @@ object WireInit { x } } + +/** RHS (source) for Invalidate API. + * Causes connection logic to emit a DefInvalid when connected to an output port (or wire). + */ +object DontCare extends Element(width = UnknownWidth()) { + // This object should be initialized before we execute any user code that refers to it, + // otherwise this "Chisel" object will end up on the UserModule's id list. + + bind(DontCareBinding(), SpecifiedDirection.Output) + override def cloneType = DontCare + + def toPrintable: Printable = PString("DONTCARE") + + private[core] def connectFromBits(that: chisel3.core.Bits)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Unit = { + Builder.error("connectFromBits: DontCare cannot be a connection sink (LHS)") + } + + def do_asUInt(implicit sourceInfo: chisel3.internal.sourceinfo.SourceInfo, compileOptions: CompileOptions): chisel3.core.UInt = { + Builder.error("DontCare does not have a UInt representation") + 0.U + } + // DontCare's only match themselves. + private[core] def typeEquivalent(that: chisel3.core.Data): Boolean = that == DontCare +} diff --git a/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala b/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala index 3c34785f..c4e880b7 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/MonoConnect.scala @@ -3,7 +3,7 @@ package chisel3.core import chisel3.internal.Builder.pushCommand -import chisel3.internal.firrtl.Connect +import chisel3.internal.firrtl.{Connect, DefInvalid} import scala.language.experimental.macros import chisel3.internal.sourceinfo.SourceInfo @@ -48,6 +48,8 @@ object MonoConnect { MonoConnectException(s": Source Record missing field ($field).") def MismatchedException(sink: String, source: String) = MonoConnectException(s": Sink ($sink) and Source ($source) have different types.") + def DontCareCantBeSink = + MonoConnectException(": DontCare cannot be a connection sink (LHS)") /** This function is what recursively tries to connect a sink and source together * @@ -89,6 +91,16 @@ object MonoConnect { case MonoConnectException(message) => throw MonoConnectException(s"($idx)$message") } } + // Handle Vec connected to DontCare. Apply the DontCare to individual elements. + case (sink_v: Vec[Data @unchecked], DontCare) => + for(idx <- 0 until sink_v.length) { + try { + implicit val compileOptions = connectCompileOptions + connect(sourceInfo, connectCompileOptions, sink_v(idx), source, context_mod) + } catch { + case MonoConnectException(message) => throw MonoConnectException(s"($idx)$message") + } + } // Handle Record case case (sink_r: Record, source_r: Record) => @@ -107,14 +119,33 @@ object MonoConnect { case MonoConnectException(message) => throw MonoConnectException(s".$field$message") } } + // Handle Record connected to DontCare. Apply the DontCare to individual elements. + case (sink_r: Record, DontCare) => + // For each field, descend with right + for((field, sink_sub) <- sink_r.elements) { + try { + connect(sourceInfo, connectCompileOptions, sink_sub, source, context_mod) + } catch { + case MonoConnectException(message) => throw MonoConnectException(s".$field$message") + } + } + // Source is DontCare - it may be connected to anything. It generates a defInvalid for the sink. + case (sink, DontCare) => pushCommand(DefInvalid(sourceInfo, sink.lref)) + // DontCare as a sink is illegal. + case (DontCare, _) => throw DontCareCantBeSink // Sink and source are different subtypes of data so fail case (sink, source) => throw MismatchedException(sink.toString, source.toString) } // This function (finally) issues the connection operation private def issueConnect(sink: Element, source: Element)(implicit sourceInfo: SourceInfo): Unit = { - pushCommand(Connect(sourceInfo, sink.lref, source.ref)) + // If the source is a DontCare, generate a DefInvalid for the sink, + // otherwise, issue a Connect. + source.binding match { + case b: DontCareBinding => pushCommand(DefInvalid(sourceInfo, sink.lref)) + case _ => pushCommand(Connect(sourceInfo, sink.lref, source.ref)) + } } // This function checks if element-level connection operation allowed. diff --git a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala index 218b27c6..c99d53cf 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala @@ -67,9 +67,15 @@ abstract class UserModule(implicit moduleCompileOptions: CompileOptions) val firrtlPorts = getModulePorts map {port => Port(port, port.specifiedDirection)} _firrtlPorts = Some(firrtlPorts) - // Generate IO invalidation commands to initialize outputs as unused - val invalidateCommands = getModulePorts map {port => DefInvalid(UnlocatableSourceInfo, port.ref)} - + // Generate IO invalidation commands to initialize outputs as unused, + // unless the client wants explicit control over their generation. + val invalidateCommands = { + if (!compileOptions.explicitInvalidate) { + getModulePorts map { port => DefInvalid(UnlocatableSourceInfo, port.ref) } + } else { + Seq() + } + } val component = DefModule(this, name, firrtlPorts, invalidateCommands ++ getCommands) _component = Some(component) component @@ -97,10 +103,11 @@ abstract class ImplicitModule(implicit moduleCompileOptions: CompileOptions) private[core] override def initializeInParent() { implicit val sourceInfo = UnlocatableSourceInfo - for (port <- getModulePorts) { - pushCommand(DefInvalid(sourceInfo, port.ref)) + if (!compileOptions.explicitInvalidate) { + for (port <- getModulePorts) { + pushCommand(DefInvalid(sourceInfo, port.ref)) + } } - clock := Builder.forcedClock reset := Builder.forcedReset } @@ -171,7 +178,9 @@ abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions) // module de-duplication in FIRRTL emission. implicit val sourceInfo = UnlocatableSourceInfo - pushCommand(DefInvalid(sourceInfo, io.ref)) + if (!compileOptions.explicitInvalidate) { + pushCommand(DefInvalid(sourceInfo, io.ref)) + } override_clock match { case Some(override_clock) => clock := override_clock diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala index 8af1835d..90eabba3 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala @@ -164,6 +164,10 @@ private[chisel3] object Builder { private def dynamicContext: DynamicContext = dynamicContextVar.value.getOrElse(new DynamicContext) + // Initialize any singleton objects before user code inadvertently inherits them. + private def initializeSingletons(): Unit = { + val dummy = DontCare + } def idGen: IdGen = dynamicContext.idGen def globalNamespace: Namespace = dynamicContext.globalNamespace def components: ArrayBuffer[Component] = dynamicContext.components @@ -245,6 +249,7 @@ private[chisel3] object Builder { Circuit(components.last.name, components, annotations.map(_.toFirrtl)) } } + initializeSingletons() } /** Allows public access to the naming stack in Builder / DynamicContext. -- cgit v1.2.3