Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve literals across .asTypeOf #4168

Merged
merged 1 commit into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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