Skip to content

Commit

Permalink
Preserve literals across .asTypeOf (#4168)
Browse files Browse the repository at this point in the history
Casting a literal (of any type) to another type with .asTypeOf will
result in a literal of the new type. For non-literals, the FIRRTL
representation will now be a little bit more efficient.
  • Loading branch information
jackkoenig authored Jul 3, 2024
1 parent d499a88 commit 8f00067
Show file tree
Hide file tree
Showing 17 changed files with 245 additions and 97 deletions.
61 changes: 44 additions & 17 deletions core/src/main/scala/chisel3/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -114,24 +114,51 @@ sealed abstract class Aggregate extends Data {
}
}

private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
var i = 0
val bits = if (that.isLit) that else WireDefault(UInt(this.width), that) // handles width padding
for (x <- flatten) {
val fieldWidth = x.getWidth
if (fieldWidth > 0) {
x.connectFromBits(bits(i + fieldWidth - 1, i))
i += fieldWidth
} else {
// There's a zero-width field in this bundle.
// Zero-width fields can't really be assigned to, but the frontend complains if there are uninitialized fields,
// so we assign it to DontCare. We can't use connectFromBits() on DontCare, so use := instead.
x := DontCare
// Return a literal of the same type from a Seq of literals for each leaf
private[chisel3] final def _makeLitFromLeaves(elems: Seq[Element])(implicit sourceInfo: SourceInfo): Data = {
// We could use virtual methods instead of matching on concrete subtypes,
// but the difference for Record vs Vec is so small, it doesn't seem worth it
val clone: Aggregate = this.cloneTypeFull
val mapping = clone.flatten.view
.zip(elems)
.map {
case (thisElt, litElt) =>
val litArg = litElt.topBindingOpt match {
case Some(ElementLitBinding(value)) => value
case _ => throwException(s"Internal Error! For field $thisElt, given non-literal $litElt!")
}
thisElt -> litArg
}
val binding = clone match {
case r: Record => BundleLitBinding(mapping.to(Map))
case v: Vec[_] => VecLitBinding(mapping.to(VectorMap))
}
clone.bind(binding)
clone
}

override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = {
val _asUInt = _resizeToWidth(that, this.widthOption)(identity)
// If that is a literal and all constituent Elements can be represented as literals, return a literal
val ((_, allLit), rvalues) = {
this.flatten.toList.mapAccumulate((0, _asUInt.isLit)) {
case ((lo, literal), elt) =>
val hi = lo + elt.getWidth
// Chisel only supports zero width extraction if hi = -1 and lo = 0, so do it manually
val _extracted = if (elt.getWidth == 0) 0.U(0.W) else _asUInt(hi - 1, lo)
// _fromUInt returns Data but we know that it is an Element
val rhs = elt._fromUInt(_extracted).asInstanceOf[Element]
((hi, literal && rhs.isLit), rhs)
}
}
if (allLit) {
this._makeLitFromLeaves(rvalues)
} else {
val _wire = Wire(this.cloneTypeFull)
for ((l, r) <- _wire.flatten.zip(rvalues)) {
l := r
}
_wire
}
}
}
Expand Down
42 changes: 14 additions & 28 deletions core/src/main/scala/chisel3/Bits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package chisel3

import scala.language.experimental.macros
import chisel3.experimental.{requireIsHardware, SourceInfo}
import chisel3.internal.{throwException, BaseModule}
import chisel3.internal.{_resizeToWidth, throwException, BaseModule}
import chisel3.internal.Builder.pushOp
import chisel3.internal.firrtl.ir._
import chisel3.internal.sourceinfo.{
Expand Down Expand Up @@ -811,12 +811,8 @@ sealed class UInt private[chisel3] (width: Width) extends Bits(width) with Num[U

override private[chisel3] def _asUIntImpl(first: Boolean)(implicit sourceInfo: SourceInfo): UInt = this

private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
this := that.asUInt
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): this.type = {
_resizeToWidth(that, this.widthOption)(identity).asInstanceOf[this.type]
}

private def subtractAsSInt(that: UInt)(implicit sourceInfo: SourceInfo): SInt =
Expand Down Expand Up @@ -1071,13 +1067,8 @@ sealed class SInt private[chisel3] (width: Width) extends Bits(width) with Num[S

override def do_asSInt(implicit sourceInfo: SourceInfo): SInt = this

private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
this := that.asSInt
}
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): this.type =
_resizeToWidth(that.asSInt, this.widthOption)(_.asSInt).asInstanceOf[this.type]
}

sealed trait Reset extends Element with ToBoolable {
Expand Down Expand Up @@ -1118,12 +1109,10 @@ final class ResetType(private[chisel3] val width: Width = Width(1)) extends Elem
DefPrim(sourceInfo, UInt(this.width), AsUIntOp, ref)
)

private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
this := that
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = {
val _wire = Wire(this.cloneTypeFull)
_wire := that
_wire
}

/** @group SourceInfoTransformMacro */
Expand Down Expand Up @@ -1162,14 +1151,7 @@ sealed class AsyncReset(private[chisel3] val width: Width = Width(1)) extends El
DefPrim(sourceInfo, UInt(this.width), AsUIntOp, ref)
)

// TODO Is this right?
private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
this := that.asBool.asAsyncReset
}
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = that.asBool.asAsyncReset

/** @group SourceInfoTransformMacro */
def do_asAsyncReset(implicit sourceInfo: SourceInfo): AsyncReset = this
Expand Down Expand Up @@ -1299,4 +1281,8 @@ sealed class Bool() extends UInt(1.W) with Reset {
/** @group SourceInfoTransformMacro */
def do_asAsyncReset(implicit sourceInfo: SourceInfo): AsyncReset =
pushOp(DefPrim(sourceInfo, AsyncReset(), AsAsyncResetOp, ref))

override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): this.type = {
_resizeToWidth(that, this.widthOption)(identity).asBool.asInstanceOf[this.type]
}
}
31 changes: 24 additions & 7 deletions core/src/main/scala/chisel3/ChiselEnum.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,8 @@ abstract class EnumType(private[chisel3] val factory: ChiselEnum, selfAnnotating
pushOp(DefPrim(sourceInfo, Bool(), op, this.ref, other.ref))
}

private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
this := factory.apply(that.asUInt)
}
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data =
factory.apply(that.asUInt)

final def ===(that: EnumType): Bool = macro SourceInfoTransform.thatArg
final def =/=(that: EnumType): Bool = macro SourceInfoTransform.thatArg
Expand All @@ -73,6 +68,28 @@ abstract class EnumType(private[chisel3] val factory: ChiselEnum, selfAnnotating
def do_>=(that: EnumType)(implicit sourceInfo: SourceInfo): Bool =
compop(sourceInfo, GreaterEqOp, that)

// This preserves the _workaround_ for https://github.com/chipsalliance/chisel/issues/4159
// Note that #4159 is due to _asUIntImpl below not actually padding the UInt
// This override just ensures that if `that` has a known width, the result actually has that width
// Put another way, this is preserving a case where #4159 does **not** occur
// This can be deleted when Builder.useLegacyWidth is removed.
override def do_asTypeOf[T <: Data](that: T)(implicit sourceInfo: SourceInfo): T = {
that.widthOption match {
// Note that default case will handle literals just fine
case Some(w) =>
val _padded = this.litOption match {
case Some(value) =>
value.U(w.W)
case None =>
val _wire = Wire(UInt(w.W))
_wire := this.asUInt
_wire
}
_padded.do_asTypeOf(that)
case None => super.do_asTypeOf(that)
}
}

override private[chisel3] def _asUIntImpl(first: Boolean)(implicit sourceInfo: SourceInfo): UInt = {
this.litOption match {
// This fixes an old bug that changes widths and thus silently changes behavior.
Expand Down
9 changes: 2 additions & 7 deletions core/src/main/scala/chisel3/Clock.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ sealed class Clock(private[chisel3] val width: Width = Width(1)) extends Element
override private[chisel3] def _asUIntImpl(first: Boolean)(implicit sourceInfo: SourceInfo): UInt = pushOp(
DefPrim(sourceInfo, UInt(this.width), AsUIntOp, ref)
)
private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
this := that.asBool.asClock
}

override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = that.asBool.asClock
}
25 changes: 8 additions & 17 deletions core/src/main/scala/chisel3/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -813,20 +813,14 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {

/** @group SourceInfoTransformMacro */
def do_asTypeOf[T <: Data](that: T)(implicit sourceInfo: SourceInfo): T = {
val thatCloned = Wire(that.cloneTypeFull)
thatCloned.connectFromBits(this.asUInt)
thatCloned.viewAsReadOnlyDeprecated(siteInfo =>
Warning(WarningID.AsTypeOfReadOnly, s"Return values of asTypeOf will soon be read-only")(siteInfo)
)
that._fromUInt(this.asUInt).asInstanceOf[T].viewAsReadOnly { _ =>
"Return values of asTypeOf are now read-only"
}
}

/** Assigns this node from Bits type. Internal implementation for asTypeOf.
/** Return a value of this type from a UInt type. Internal implementation for asTypeOf.
*/
private[chisel3] def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit
private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data

/** Reinterpret cast to UInt.
*
Expand Down Expand Up @@ -1213,12 +1207,9 @@ final case object DontCare extends Element with connectable.ConnectableDocs {

def toPrintable: Printable = PString("DONTCARE")

private[chisel3] def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
Builder.error("connectFromBits: DontCare cannot be a connection sink (LHS)")
private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = {
Builder.error("DontCare cannot be a connection sink (LHS)")
this
}

override private[chisel3] def _asUIntImpl(first: Boolean)(implicit sourceInfo: SourceInfo): UInt = {
Expand Down
11 changes: 4 additions & 7 deletions core/src/main/scala/chisel3/experimental/Analog.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

package chisel3.experimental

import chisel3._
import chisel3.internal._
import chisel3.internal.binding._
import chisel3.{ActualDirection, Bits, Data, Element, PString, Printable, RawModule, SpecifiedDirection, UInt, Width}

import scala.collection.mutable

Expand Down Expand Up @@ -70,12 +70,9 @@ final class Analog private (private[chisel3] val width: Width) extends Element {
override private[chisel3] def _asUIntImpl(first: Boolean)(implicit sourceInfo: SourceInfo): UInt =
throwException("Analog does not support asUInt")

private[chisel3] override def connectFromBits(
that: Bits
)(
implicit sourceInfo: SourceInfo
): Unit = {
throwException("Analog does not support connectFromBits")
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = {
Builder.error("Analog does not support fromUInt")
Wire(Analog(that.width))
}

def toPrintable: Printable = PString("Analog")
Expand Down
36 changes: 36 additions & 0 deletions core/src/main/scala/chisel3/internal/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import scala.annotation.implicitNotFound
import scala.collection.mutable
import chisel3.ChiselException

import scala.reflect.runtime.universe.{typeTag, TypeTag}

package object internal {

@implicitNotFound("You are trying to access a macro-only API. Please use the @public annotation instead.")
Expand Down Expand Up @@ -67,6 +69,40 @@ package object internal {
if (headOk) res else s"_$res"
}

// Workaround for https://github.com/chipsalliance/chisel/issues/4162
// We can't use the .asTypeOf workaround because this is used to implement .asTypeOf
private[chisel3] def _padHandleBool[A <: Bits](
x: A,
width: Int
)(
implicit sourceInfo: SourceInfo,
tag: TypeTag[A]
): A = x match {
case b: Bool if !b.isLit && width > 1 && tag.tpe =:= typeTag[UInt].tpe =>
val _pad = Wire(UInt(width.W))
_pad := b
_pad.asInstanceOf[A] // This cast is safe because we know A is UInt on this path
case u => u.pad(width)
}

// Resize that to this width (if known)
private[chisel3] def _resizeToWidth[A <: Bits: TypeTag](
that: A,
targetWidthOpt: Option[Int]
)(fromUInt: UInt => A
)(
implicit sourceInfo: SourceInfo
): A =
(targetWidthOpt, that.widthOption) match {
case (Some(targetWidth), Some(thatWidth)) =>
if (targetWidth == thatWidth) that
else if (targetWidth > thatWidth) _padHandleBool(that, targetWidth)
else fromUInt(that.take(targetWidth))
case (Some(targetWidth), None) => fromUInt(_padHandleBool(that, targetWidth).take(targetWidth))
case (None, Some(thatWidth)) => that
case (None, None) => that
}

/** Internal API for [[ViewParent]] */
sealed private[chisel3] class ViewParentAPI extends RawModule() with PseudoModule {
// We must provide `absoluteTarget` but not `toTarget` because otherwise they would be exactly
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/scala/chisel3/properties/Property.scala
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,10 @@ sealed trait Property[T] extends Element { self =>
Builder.error(s"${this._localErrorContext} does not support .asUInt.")
0.U
}
private[chisel3] def connectFromBits(that: Bits)(implicit sourceInfo: SourceInfo): Unit = {
Builder.error(s"${this._localErrorContext} cannot be driven by Bits")
override private[chisel3] def _fromUInt(that: UInt)(implicit sourceInfo: SourceInfo): Data = {
Builder.exception(s"${this._localErrorContext} cannot be driven by UInt")
}

override private[chisel3] def firrtlConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit = {
that match {
case pthat: Property[_] => MonoConnect.propConnect(sourceInfo, this, pthat, Builder.forcedUserModule)
Expand Down
6 changes: 6 additions & 0 deletions docs/src/explanations/warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ field's width).

### [W008] Return values of asTypeOf will soon be read-only

:::warning

As of Chisel 7.0.0, this is now an error

:::

This warning indicates that the result of a call to `.asTypeOf(_)` is being used as the destination for a connection.
It can be fixed by instantiating a wire.

Expand Down
15 changes: 9 additions & 6 deletions src/test/scala/chiselTests/AsTypeOfTester.scala
Original file line number Diff line number Diff line change
Expand Up @@ -477,12 +477,15 @@ class AsTypeOfSpec extends ChiselFunSpec {
describe("Analogs") {
describe("as the target type") {
they("should error") {
val e = the[ChiselException] thrownBy ChiselStage.emitSystemVerilog(new RawModule {
val in = IO(Input(UInt(8.W)))
val out = IO(Analog(8.W))
out := in.asTypeOf(out)
})
e.getMessage should include("Analog does not support connectFromBits")
val e = the[ChiselException] thrownBy ChiselStage.emitSystemVerilog(
new RawModule {
val in = IO(Input(UInt(8.W)))
val out = IO(Analog(8.W))
out := in.asTypeOf(out)
},
Array("--throw-on-first-error")
)
e.getMessage should include("Analog does not support fromUInt")
}
}
describe("as the source type") {
Expand Down
Loading

0 comments on commit 8f00067

Please sign in to comment.