Skip to content

Commit

Permalink
Make Property an Element.
Browse files Browse the repository at this point in the history
This is the right place in the Chisel type hierarchy for Property as
it exists today. Property types can be used in Aggregates, and are not
themselves Aggregates in their current form. Making Property extend
Element allows much of Chisel's internal reflection to work as
expected when Property types are used in Aggregates.
  • Loading branch information
mikeurbach committed Dec 5, 2023
1 parent b921c4e commit f90d011
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package chisel3.experimental

import chisel3._
import chisel3.internal._
import chisel3.properties.Property

import scala.annotation.{implicitNotFound, tailrec}
import scala.collection.mutable
Expand Down Expand Up @@ -145,6 +146,8 @@ package object dataview {
case (_: Reset, a: AsyncReset) =>
/* Allow DontCare in the target only */
case (DontCare, _) =>
/* Allow Property[_] <=> Property[_] views when the underlying type is the same */
case (a: Property[_], b: Property[_]) if a.getPropertyType == b.getPropertyType =>
/* All other views produce a runtime error. */
case _ =>
val fieldName = viewFieldName(vex)
Expand Down
3 changes: 0 additions & 3 deletions core/src/main/scala/chisel3/internal/BiConnect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@ private[chisel3] object BiConnect {
elemConnect(sourceInfo, left_e, right_e, context_mod)
// TODO(twigg): Verify the element-level classes are connectable
}
case (left_p: Property[_], right_p: Property[_]) =>
// We can consider supporting this, but it's a lot of code and we should be moving away from <> anyway
Builder.error(s"${left_p._localErrorContext} does not support <>, use :<>= instead")(sourceInfo)
// Handle Vec case
case (left_v: Vec[Data @unchecked], right_v: Vec[Data @unchecked]) => {
if (left_v.length != right_v.length) {
Expand Down
23 changes: 12 additions & 11 deletions core/src/main/scala/chisel3/internal/MonoConnect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,14 @@ private[chisel3] object MonoConnect {

def propConnect(
sourceInfo: SourceInfo,
sink: Property[_],
source: Property[_],
sinkProp: Property[_],
sourceProp: Property[_],
context: BaseModule
): Unit = {
// Reify sink and source if they're views.
val sink = reify(sinkProp)
val source = reify(sourceProp)

checkConnect(sourceInfo, sink, source, context)
// Add the PropAssign command directly onto the correct BaseModule subclass.
context match {
Expand Down Expand Up @@ -447,16 +451,13 @@ private[chisel3] object checkConnect {
context_mod: BaseModule
): Unit = {
checkConnection(sourceInfo, sink, source, context_mod)
}

def apply(
sourceInfo: SourceInfo,
sink: Property[_],
source: Property[_],
context_mod: BaseModule
): Unit = {
checkConnection(sourceInfo, sink, source, context_mod)
checkConnect.checkPropertyConnection(sourceInfo, sink, source)
// Extra checks for Property[_] elements.
(sink, source) match {
case (sinkProp: Property[_], sourceProp: Property[_]) =>
checkConnect.checkPropertyConnection(sourceInfo, sinkProp, sourceProp)
case (_, _) =>
}
}

def checkConnection(
Expand Down
16 changes: 9 additions & 7 deletions core/src/main/scala/chisel3/properties/Property.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import firrtl.{ir => fir}
import firrtl.annotations.{InstanceTarget, IsMember, ModuleTarget, ReferenceTarget, Target}
import chisel3.internal._
import chisel3.internal.{firrtl => ir}
import chisel3.experimental.{prefix, requireIsHardware, SourceInfo}
import chisel3.experimental.{prefix, requireIsHardware, Analog, SourceInfo}
import chisel3.experimental.hierarchy.Instance
import scala.reflect.runtime.universe.{typeOf, TypeTag}
import scala.annotation.{implicitAmbiguous, implicitNotFound}
Expand Down Expand Up @@ -170,10 +170,13 @@ private[chisel3] object PropertyType extends LowPriorityPropertyTypeInstances {
override def convertUnderlying(value: D) = Path(value)
}

// We can't just do <: Data because Property subclasses Data
// We can't just do <: Element because Property subclasses Element
implicit def aggregatePathTypeInstance[A <: Aggregate]: RecursivePropertyType.Aux[A, Path, Path] =
dataPathTypeInstance[A]
implicit def elementPathTypeInstance[E <: Element]: RecursivePropertyType.Aux[E, Path, Path] = dataPathTypeInstance[E]
implicit def bitsPathTypeInstance[E <: ToBoolable]: RecursivePropertyType.Aux[E, Path, Path] = dataPathTypeInstance[E]
implicit def clockPathTypeInstance[E <: Clock]: RecursivePropertyType.Aux[E, Path, Path] = dataPathTypeInstance[E]
implicit def analogPathTypeInstance[E <: Analog]: RecursivePropertyType.Aux[E, Path, Path] = dataPathTypeInstance[E]
implicit def enumPathTypeInstance[E <: EnumType]: RecursivePropertyType.Aux[E, Path, Path] = dataPathTypeInstance[E]

implicit def memPathTypeInstance[M <: MemBase[_]]: RecursivePropertyType.Aux[M, Path, Path] =
new RecursivePropertyType[M] {
Expand Down Expand Up @@ -208,7 +211,7 @@ private[chisel3] object PropertyType extends LowPriorityPropertyTypeInstances {
* describe a set of non-hardware types, so they have no width, cannot be used
* in aggregate Data types, and cannot be connected to Data types.
*/
sealed trait Property[T] extends Data { self =>
sealed trait Property[T] extends Element { self =>
sealed trait ClassType
private object ClassType {
implicit def classTypeProvider(
Expand All @@ -231,19 +234,18 @@ sealed trait Property[T] extends Data { self =>
Builder.error(s"${this._localErrorContext} does not support .asUInt.")
0.U
}
private[chisel3] def allElements: Seq[Element] = Nil
private[chisel3] def connectFromBits(that: Bits)(implicit sourceInfo: SourceInfo): Unit = {
Builder.error(s"${this._localErrorContext} cannot be driven by Bits")
}
private[chisel3] def firrtlConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = {
override private[chisel3] def firrtlConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = {
that match {
case pthat: Property[_] => MonoConnect.propConnect(sourceInfo, this, pthat, Builder.forcedUserModule)
case other => Builder.error(s"${this._localErrorContext} cannot be connected to ${that._localErrorContext}")
}

}

def litOption: Option[BigInt] = None
override def litOption: Option[BigInt] = None
def toPrintable: Printable = {
throwException(s"Properties do not support hardware printing" + this._errorContext)
}
Expand Down
36 changes: 20 additions & 16 deletions src/test/scala/chiselTests/properties/PropertySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package chiselTests.properties

import chisel3._
import chisel3.experimental.FlatIO
import chisel3.properties.{Class, Path, Property, PropertyType}
import chiselTests.{ChiselFlatSpec, MatchesAndOmits}
import circt.stage.ChiselStage
Expand Down Expand Up @@ -407,22 +408,6 @@ class PropertySpec extends ChiselFlatSpec with MatchesAndOmits {
)()
}

it should "NOT support <>" in {
class MyBundle extends Bundle {
val foo = Property[String]()
val bar = Flipped(Property[BigInt]())
}
val e = the[ChiselException] thrownBy ChiselStage.emitCHIRRTL(
new RawModule {
val aligned = IO(new MyBundle)
val flipped = IO(Flipped(new MyBundle))
aligned <> flipped
},
Array("--throw-on-first-error")
)
e.getMessage should include("Field '_.bar' of type Property[Integer] does not support <>, use :<>= instead")
}

it should "support being nested in a Bundle in a wire" in {
class MyBundle extends Bundle {
val foo = Property[String]()
Expand Down Expand Up @@ -584,4 +569,23 @@ class PropertySpec extends ChiselFlatSpec with MatchesAndOmits {
"propassign barB, barObj"
)()
}

it should "support FlatIO" in {
val chirrtl = ChiselStage.emitCHIRRTL(new RawModule {
val flatModule = Module(new RawModule {
val io = FlatIO(new Bundle {
val bool = Input(Bool())
val prop = Input(Property[Int]())
})
})

flatModule.io.bool := true.B
flatModule.io.prop := Property(1)
})

matchesAndOmits(chirrtl)(
"connect flatModule.bool, UInt<1>(0h1)",
"propassign flatModule.prop, Integer(1)"
)()
}
}

0 comments on commit f90d011

Please sign in to comment.