From 156765e6b41794a45677efb21944cd39dcf283a2 Mon Sep 17 00:00:00 2001 From: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com> Date: Sat, 13 Aug 2022 09:58:36 -0700 Subject: [PATCH] Add OpaqueType support to Records (#2662) OpaqueTypes are essentially type aliases that hide the underlying type. They are implemented in Chisel as Records of a single, unnamed element where the wrapping Record does not exist in the emitted FIRRTL. Co-authored-by: Jack Koenig (cherry picked from commit df5afee2d41b5bcd82d4013b977965c0dfe046fd) --- core/src/main/scala/chisel3/Aggregate.scala | 22 ++++- .../main/scala/chisel3/internal/Builder.scala | 11 ++- .../chisel3/internal/firrtl/Converter.scala | 10 ++- .../scala/chisel3/internal/firrtl/IR.scala | 6 ++ src/test/scala/chiselTests/RecordSpec.scala | 84 +++++++++++++++++++ 5 files changed, 127 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 82c02cbc386..042e78b1243 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -923,6 +923,20 @@ trait VecLike[T <: Data] extends IndexedSeq[T] with HasId with SourceInfoDoc { */ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptions) extends Aggregate { + /** Indicates if this Record represents an "Opaque Type" + * + * Opaque types provide a mechanism for user-defined types + * that do not impose any "boxing" overhead in the emitted FIRRTL and Verilog. + * You can think about an opaque type Record as a box around + * a single element that only exists at Chisel elaboration time. + * Put another way, if opaqueType is overridden to true, + * The Record may only contain a single element with an empty name + * and there will be no `_` in the name for that element in the emitted Verilog. + * + * @see RecordSpec in Chisel's tests for example usage and expected output + */ + def opaqueType: Boolean = false + // Doing this earlier than onModuleClose allows field names to be available for prefixing the names // of hardware created when connecting to one of these elements private def setElementRefs(): Unit = { @@ -930,7 +944,13 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio // identifier; however, Namespace sanitizes identifiers to make them legal for Firrtl/Verilog // which can cause collisions val _namespace = Namespace.empty - for ((name, elt) <- elements) { elt.setRef(this, _namespace.name(name, leadingDigitOk = true)) } + require( + !opaqueType || (elements.size == 1 && elements.head._1 == ""), + s"Opaque types must have exactly one element with an empty name, not ${elements.size}: ${elements.keys.mkString(", ")}" + ) + for ((name, elt) <- elements) { + elt.setRef(this, _namespace.name(name, leadingDigitOk = true), opaque = opaqueType) + } } private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = { diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 75d08f92d3f..07d8e7f7542 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -231,9 +231,13 @@ private[chisel3] trait HasId extends InstanceId { _refVar = imm } } - private[chisel3] def setRef(parent: HasId, name: String): Unit = setRef(Slot(Node(parent), name)) - private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index))) - private[chisel3] def setRef(parent: HasId, index: UInt): Unit = setRef(Index(Node(parent), index.ref)) + private[chisel3] def setRef(parent: HasId, name: String, opaque: Boolean = false): Unit = { + if (!opaque) setRef(Slot(Node(parent), name)) + else setRef(OpaqueSlot(Node(parent), name)) + } + + private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index))) + private[chisel3] def setRef(parent: HasId, index: UInt): Unit = setRef(Index(Node(parent), index.ref)) private[chisel3] def getRef: Arg = _ref.get private[chisel3] def getOptionRef: Option[Arg] = _ref @@ -514,6 +518,7 @@ private[chisel3] object Builder extends LazyLogging { def buildAggName(id: HasId): Option[String] = { def getSubName(field: Data): Option[String] = field.getOptionRef.flatMap { case Slot(_, field) => Some(field) // Record + case OpaqueSlot(_, field) => None // Record with single element case Index(_, ILit(n)) => Some(n.toString) // Vec static indexing case Index(_, ULit(n, _)) => Some(n.toString) // Vec lit indexing case Index(_, _: Node) => None // Vec dynamic indexing diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 2d21a7cbed4..56422b8569f 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -68,6 +68,8 @@ private[chisel3] object Converter { fir.Reference(name, fir.UnknownType) case Slot(imm, name) => fir.SubField(convert(imm, ctx, info), name, fir.UnknownType) + case OpaqueSlot(imm, name) => + convert(imm, ctx, info) case Index(imm, ILit(idx)) => fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType) case Index(imm, value) => @@ -301,7 +303,7 @@ private[chisel3] object Converter { case d: Interval => fir.IntervalType(d.range.lowerBound, d.range.upperBound, d.range.firrtlBinaryPoint) case d: Analog => fir.AnalogType(convert(d.width)) case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir, info), d.length) - case d: Record => + case d: Record => { val childClearDir = clearDir || d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output def eltField(elt: Data): fir.Field = (childClearDir, firrtlUserDirOf(elt)) match { @@ -311,7 +313,11 @@ private[chisel3] object Converter { case (false, SpecifiedDirection.Flip | SpecifiedDirection.Input) => fir.Field(getRef(elt, info).name, fir.Flip, extractType(elt, false, info)) } - fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) + if (!d.opaqueType) + fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) + else + extractType(d.elements.head._2, true, info) + } } def convert(name: String, param: Param): fir.Param = param match { diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index dc9ab027b6f..37fb2f8b508 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -91,6 +91,7 @@ object Arg { case Some(Index(Node(imm), Node(value))) => s"${earlyLocalName(imm)}[${earlyLocalName(imm)}]" case Some(Index(Node(imm), arg)) => s"${earlyLocalName(imm)}[${arg.localName}]" case Some(Slot(Node(imm), name)) => s"${earlyLocalName(imm)}.$name" + case Some(OpaqueSlot(Node(imm), name)) => s"${earlyLocalName(imm)}" case Some(arg) => arg.name case None => id match { @@ -218,6 +219,11 @@ case class Slot(imm: Node, name: String) extends Arg { } } +case class OpaqueSlot(imm: Node, name: String) extends Arg { + override def contextualName(ctx: Component): String = imm.name + override def localName: String = imm.name +} + case class Index(imm: Arg, value: Arg) extends Arg { def name: String = s"[$value]" override def contextualName(ctx: Component): String = s"${imm.contextualName(ctx)}[${value.contextualName(ctx)}]" diff --git a/src/test/scala/chiselTests/RecordSpec.scala b/src/test/scala/chiselTests/RecordSpec.scala index 30b55812c50..a18c2518a70 100644 --- a/src/test/scala/chiselTests/RecordSpec.scala +++ b/src/test/scala/chiselTests/RecordSpec.scala @@ -108,6 +108,58 @@ trait RecordSpecUtils { require(DataMirror.checkTypeEquivalence(wire0, wire1)) require(!DataMirror.checkTypeEquivalence(wire1, wire2)) } + + class SingleElementRecord extends Record { + private val underlying = UInt(8.W) + val elements = SeqMap("" -> underlying) + override def opaqueType = elements.size == 1 + override def cloneType: this.type = this + + def +(that: SingleElementRecord): SingleElementRecord = { + val _w = Wire(new SingleElementRecord) + _w.underlying := this.underlying + that.underlying + _w + } + } + + class SingleElementRecordModule extends Module { + val in1 = IO(Input(new SingleElementRecord)) + val in2 = IO(Input(new SingleElementRecord)) + val out = IO(Output(new SingleElementRecord)) + + val r = new SingleElementRecord + + out := in1 + in2 + } + + class NamedSingleElementRecord extends Record { + private val underlying = UInt(8.W) + val elements = SeqMap("unused" -> underlying) + + override def opaqueType = elements.size == 1 + override def cloneType: this.type = this + } + + class NamedSingleElementModule extends Module { + val in = IO(Input(new NamedSingleElementRecord)) + val out = IO(Output(new NamedSingleElementRecord)) + out := in + } + + class ErroneousOverride extends Record { + private val underlyingA = UInt(8.W) + private val underlyingB = UInt(8.W) + val elements = SeqMap("x" -> underlyingA, "y" -> underlyingB) + + override def opaqueType = true + override def cloneType: this.type = this + } + + class ErroneousOverrideModule extends Module { + val in = IO(Input(new ErroneousOverride)) + val out = IO(Output(new ErroneousOverride)) + out := in + } } class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils { @@ -146,6 +198,38 @@ class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils { e.getMessage should include("contains aliased fields named (bar,foo)") } + they should "be OpaqueType for maps with single unnamed elements" in { + val singleElementChirrtl = ChiselStage.emitChirrtl { new SingleElementRecordModule } + singleElementChirrtl should include("input in1 : UInt<8>") + singleElementChirrtl should include("input in2 : UInt<8>") + singleElementChirrtl should include("add(in1, in2)") + } + + they should "throw an error when map contains a named element and opaqueType is overriden to true" in { + (the[Exception] thrownBy extractCause[Exception] { + ChiselStage.elaborate { new NamedSingleElementModule } + }).getMessage should include("Opaque types must have exactly one element with an empty name") + } + + they should "throw an error when map contains more than one element and opaqueType is overriden to true" in { + (the[Exception] thrownBy extractCause[Exception] { + ChiselStage.elaborate { new ErroneousOverrideModule } + }).getMessage should include("Opaque types must have exactly one element with an empty name") + } + + they should "work with .toTarget" in { + var m: SingleElementRecordModule = _ + ChiselStage.elaborate { m = new SingleElementRecordModule; m } + val q = m.in1.toTarget.toString + assert(q == "~SingleElementRecordModule|SingleElementRecordModule>in1") + } + + they should "NOT work with .toTarget on non-data OpaqueType Record" in { + var m: SingleElementRecordModule = _ + ChiselStage.elaborate { m = new SingleElementRecordModule; m } + a[ChiselException] shouldBe thrownBy { m.r.toTarget } + } + they should "follow UInt serialization/deserialization API" in { assertTesterPasses { new RecordSerializationTest } }