Skip to content

Commit

Permalink
Add support for marking things as readOnly
Browse files Browse the repository at this point in the history
Also add internal support for marking things as "deprecated read only".
  • Loading branch information
jackkoenig committed Jun 18, 2024
1 parent 9d2f156 commit 406d936
Show file tree
Hide file tree
Showing 16 changed files with 742 additions and 111 deletions.
4 changes: 2 additions & 2 deletions core/src/main/scala/chisel3/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ sealed abstract class Aggregate extends Data {
// Don't accidentally invent a literal value for a view that is empty
case Some(_: AggregateViewBinding) if this.getElements.isEmpty =>
reifyIdentityView(this) match {
case Some(target: Aggregate) => target.checkingLitOption(checkForDontCares)
case Some((target: Aggregate, _)) => target.checkingLitOption(checkForDontCares)
// This can occur with empty Vecs or Bundles
case _ => None
}
Expand Down Expand Up @@ -355,7 +355,7 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend
// Views complicate things a bit, but views that correspond exactly to an identical Vec can just forward the
// dynamic indexing to the target Vec
// In theory, we could still do this forwarding if the sample element were different by deriving a DataView
case Some(target: Vec[T @unchecked])
case Some((target: Vec[T @unchecked], _))
if this.length == target.length &&
this.sample_element.typeEquivalent(target.sample_element) =>
return target.apply(p)
Expand Down
33 changes: 25 additions & 8 deletions core/src/main/scala/chisel3/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import chisel3.experimental.dataview.reify
import scala.language.experimental.macros
import chisel3.experimental.{requireIsChiselType, requireIsHardware, Analog, BaseModule}
import chisel3.experimental.{prefix, SourceInfo, UnlocatableSourceInfo}
import chisel3.experimental.dataview.{reifyIdentityView, reifySingleTarget}
import chisel3.experimental.dataview.{reifyIdentityView, reifySingleTarget, DataViewable}
import chisel3.internal.Builder.pushCommand
import chisel3.internal._
import chisel3.internal.binding._
Expand Down Expand Up @@ -629,8 +629,8 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
case Some(pb: PortBinding)
if mod.flatMap(Builder.retrieveParent(_, Builder.currentModule.get)) == Builder.currentModule =>
true
case Some(ViewBinding(target)) => target.isVisibleFromModule
case Some(AggregateViewBinding(mapping)) => mapping.values.forall(_.isVisibleFromModule)
case Some(ViewBinding(target, _)) => target.isVisibleFromModule
case Some(AggregateViewBinding(mapping, _)) => mapping.values.forall(_.isVisibleFromModule)
case Some(pb: SecretPortBinding) => true // Ignore secret to not require visibility
case Some(_: UnconstrainedBinding) => true
case _ => false
Expand All @@ -651,13 +651,16 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
}

// Internal API: returns a ref that can be assigned to, if consistent with the binding
private[chisel3] def lref: Node = {
private[chisel3] def lref(implicit info: SourceInfo): Node = {
requireIsHardware(this)
requireVisible()
topBindingOpt match {
case Some(binding: ReadOnlyBinding) =>
throwException(s"internal error: attempted to generate LHS ref to ReadOnlyBinding $binding")
case Some(ViewBinding(target)) => reify(target).lref
case Some(ViewBinding(target1, wr1)) =>
val (target2, wr2) = reify(target1)
val writability = wr1.combine(wr2)
writability.reportIfReadOnly(target2.lref)(Wire(chiselTypeOf(target2)).lref)
case Some(binding: TopBinding) => Node(this)
case opt => throwException(s"internal error: unknown binding $opt in generating LHS ref")
}
Expand All @@ -677,11 +680,11 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
requireIsHardware(this)
topBindingOpt match {
// DataView
case Some(ViewBinding(target)) => reify(target).ref
case Some(ViewBinding(target, _)) => reify(target)._1.ref
case Some(_: AggregateViewBinding) =>
reifyIdentityView(this) match {
// If this is an identity view (a view of something of the same type), return ref of target
case Some(target) => target.ref
case Some((target, _)) => target.ref
// Otherwise, we need to materialize hardware of the correct type
case _ => materializeWire()
}
Expand Down Expand Up @@ -812,7 +815,9 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
def do_asTypeOf[T <: Data](that: T)(implicit sourceInfo: SourceInfo): T = {
val thatCloned = Wire(that.cloneTypeFull)
thatCloned.connectFromBits(this.asUInt)
thatCloned
thatCloned.viewAsReadOnlyDeprecated(siteInfo =>
Warning(WarningID.AsTypeOfReadOnly, s"Return values of asTypeOf will soon be read-only")(siteInfo)
)
}

/** Assigns this node from Bits type. Internal implementation for asTypeOf.
Expand Down Expand Up @@ -1017,6 +1022,18 @@ object Data {
}
}
}

implicit class AsReadOnly[T <: Data](self: T) {

/** Returns a read-only view of this Data
*
* It is illegal to connect to the return value of this method.
* This Data this method is called on must be a hardware type.
*/
def readOnly(implicit sourceInfo: SourceInfo): T = {
self.viewAsReadOnly(_ => "Cannot connect to read-only value")
}
}
}

trait WireFactory {
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/scala/chisel3/Element.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ abstract class Element extends Data {
case _ => Some(DontCareBinding())
}
// TODO Do we even need this? Looking up things in the AggregateViewBinding is fine
case Some(b @ AggregateViewBinding(viewMap)) =>
case Some(b @ AggregateViewBinding(viewMap, _)) =>
viewMap.get(this) match {
case Some(elt: Element) => Some(ViewBinding(elt))
case Some(elt: Element) =>
// Very important to use this instead of elt as "this" is the key to the viewMap
val wr = b.lookupWritability(this)
Some(ViewBinding(elt, wr))
// Children of Probes won't be in viewMap, just return the binding
case _ => Some(b)
}
Expand All @@ -51,7 +54,7 @@ abstract class Element extends Data {
private[chisel3] def litArgOption: Option[LitArg] = topBindingOpt match {
case Some(ElementLitBinding(litArg)) => Some(litArg)
case Some(_: ViewBinding) =>
reify(this).litArgOption
reify(this)._1.litArgOption
case _ => None
}

Expand Down
98 changes: 72 additions & 26 deletions core/src/main/scala/chisel3/experimental/dataview/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,21 @@ package object dataview {
* Calling `viewAs` also requires an implementation of [[DataView]] for the target type
*/
implicit class DataViewable[T](target: T) {
def viewAs[V <: Data](implicit dataproduct: DataProduct[T], dataView: DataView[T, V], sourceInfo: SourceInfo): V = {
private def _viewAsImpl[V <: Data](
writability: ViewWriteability
)(
implicit dataproduct: DataProduct[T],
dataView: DataView[T, V],
sourceInfo: SourceInfo
): V = {
// TODO put a try catch here for ExpectedHardwareException and perhaps others
// It's likely users will accidentally use chiselTypeOf or something that may error,
// The right thing to use is DataMirror...chiselTypeClone because of composition with DataView.andThen
// Another option is that .andThen could give a fake binding making chiselTypeOfs in the user code safe
val result: V = dataView.mkView(target)
requireIsChiselType(result, "viewAs")

doBind(target, result, dataView)
doBind(target, result, dataView, writability)

// Setting the parent marks these Data as Views
result.setAllParents(Some(ViewParent))
Expand All @@ -38,6 +44,28 @@ package object dataview {
result.forceName("view", Builder.viewNamespace)
result
}

def viewAs[V <: Data](
implicit dataproduct: DataProduct[T],
dataView: DataView[T, V],
sourceInfo: SourceInfo
): V = _viewAsImpl(ViewWriteability.Default)

private[chisel3] def viewAsReadOnlyDeprecated[V <: Data](
getWarning: SourceInfo => Warning
)(
implicit dataproduct: DataProduct[T],
dataView: DataView[T, V],
sourceInfo: SourceInfo
): V = _viewAsImpl(ViewWriteability.ReadOnlyDeprecated(getWarning))

private[chisel3] def viewAsReadOnly[V <: Data](
getError: SourceInfo => String
)(
implicit dataproduct: DataProduct[T],
dataView: DataView[T, V],
sourceInfo: SourceInfo
): V = _viewAsImpl(ViewWriteability.ReadOnly(getError))
}

/** Provides `viewAsSupertype` for subclasses of [[Record]] */
Expand Down Expand Up @@ -75,9 +103,10 @@ package object dataview {

// TODO should this be moved to class Aggregate / can it be unified with Aggregate.bind?
private def doBind[T: DataProduct, V <: Data](
target: T,
view: V,
dataView: DataView[T, V]
target: T,
view: V,
dataView: DataView[T, V],
writability: ViewWriteability
)(
implicit sourceInfo: SourceInfo
): Unit = {
Expand Down Expand Up @@ -205,7 +234,7 @@ package object dataview {
}

view match {
case elt: Element => view.bind(ViewBinding(elementResult(elt)))
case elt: Element => view.bind(ViewBinding(elementResult(elt), writability))
case agg: Aggregate =>
val fullResult = elementResult ++ aggregateMappings

Expand All @@ -222,15 +251,19 @@ package object dataview {
case _ => // Do nothing
}
}
agg.bind(AggregateViewBinding(fullResult))
val aggWritability = Option.when(writability.isReadOnly)(
Map((agg: Data) -> writability)
)
agg.bind(AggregateViewBinding(fullResult, aggWritability))
}
}

// Traces an Element that may (or may not) be a view until it no longer maps
// Inclusive of the argument
// Note that this does *not* include writability so do not use this in place of reify
private def unfoldView(elt: Element): LazyList[Element] = {
def rec(e: Element): LazyList[Element] = e.topBindingOpt match {
case Some(ViewBinding(target)) => target #:: rec(target)
case Some(ViewBinding(target, _)) => target #:: rec(target)
case Some(avb: AggregateViewBinding) =>
val target = avb.lookup(e).get
target #:: rec(target)
Expand All @@ -247,19 +280,25 @@ package object dataview {
* This is the fundamental "unwrapping" or "tracing" primitive operation for handling Views within
* Chisel.
*/
private[chisel3] def reify(elt: Element): Element =
reify(elt, elt.topBinding)
private[chisel3] def reify(elt: Element): (Element, ViewWriteability) =
reify(elt, elt.topBinding, ViewWriteability.Default)

/** Turn any [[Element]] that could be a View into a concrete Element
*
* This is the fundamental "unwrapping" or "tracing" primitive operation for handling Views within
* Chisel.
*/
@tailrec private[chisel3] def reify(elt: Element, topBinding: TopBinding): Element =
@tailrec private[chisel3] def reify(
elt: Element,
topBinding: TopBinding,
wrAcc: ViewWriteability
): (Element, ViewWriteability) = {
topBinding match {
case ViewBinding(target) => reify(target, target.topBinding)
case _ => elt
case ViewBinding(target, writeability) =>
reify(target, target.topBinding, wrAcc.combine(writeability))
case _ => (elt, wrAcc)
}
}

/** Determine the target of a View if the view is an identity mapping.
*
Expand All @@ -269,19 +308,24 @@ package object dataview {
* @return The identity target of this view or None if not an identity view.
* @note Returns Some(_) of the argument if it is not a view.
*/
private[chisel3] def reifyIdentityView[T <: Data](data: T): Option[T] = {
val candidate: Option[Data] =
private[chisel3] def reifyIdentityView[T <: Data](
data: T,
wrAcc: ViewWriteability = ViewWriteability.Default
): Option[(T, ViewWriteability)] = {
val candidate: Option[(Data, ViewWriteability)] =
data.topBindingOpt match {
case None => None
case Some(ViewBinding(target)) => Some(target)
case Some(AggregateViewBinding(lookup)) => lookup.get(data)
case Some(_) => Some(data)
case None => None
case Some(ViewBinding(target, wr)) => Some(target -> wr)
case Some(vb @ AggregateViewBinding(lookup, _)) => lookup.get(data).map(_ -> vb.lookupWritability(data))
case Some(_) => Some(data -> ViewWriteability.Default)
}
candidate.flatMap { d =>
// This cast is safe by construction, we only put Data in the view mapping if it is an identity mapping
val cast = d.asInstanceOf[T]
// Candidate may itself be a view, keep tracing in those cases
if (isView(d)) reifyIdentityView(cast) else Some(cast)
candidate.flatMap {
case (d, wr) =>
val wrx = wrAcc.combine(wr)
// This cast is safe by construction, we only put Data in the view mapping if it is an identity mapping
val cast = d.asInstanceOf[T]
// Candidate may itself be a view, keep tracing in those cases
if (isView(d)) reifyIdentityView(cast, wrx) else Some(cast -> wrx)
}
}

Expand All @@ -298,15 +342,17 @@ package object dataview {
*
* @return The single Data target of this view or None if a single Data doesn't exist.
* @note Returns Some(_) of the argument if it is not a view.
* @note You should never attempt to write the result of this function.
*/
private[chisel3] def reifySingleTarget(data: Data): Option[Data] = {
def err(msg: String) = throwException(s"Internal Error! $msg reifySingleTarget($data)")
// Identity views are obviously single targets.
reifyIdentityView(data).orElse {
// We ignore writability because the return of this function should never be written.
reifyIdentityView(data).map(_._1).orElse {
// Otherwise, all children of data need to map to all of the children of another Data.
// This is really expensive, is there a better way?
data.topBindingOpt.flatMap {
case AggregateViewBinding(mapping) =>
case AggregateViewBinding(mapping, _) =>
// Take every single leaf and map to its target
val leaves = DataMirror.collectLeafMembers(data)
val targets = leaves.map { l =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import chisel3._
import chisel3.experimental.dataview.{isView, reify, reifyIdentityView}
import chisel3.internal.firrtl.ir.{Arg, ILit, Index, ModuleIO, Slot, ULit}
import chisel3.internal.{throwException, Builder, ViewParent}
import chisel3.internal.binding.{AggregateViewBinding, ChildBinding, CrossModuleBinding, ViewBinding}
import chisel3.internal.binding.{AggregateViewBinding, ChildBinding, CrossModuleBinding, ViewBinding, ViewWriteability}

/** Represents lookup typeclass to determine how a value accessed from an original IsInstantiable
* should be tweaked to return the Instance's version
Expand Down Expand Up @@ -176,19 +176,25 @@ object Lookupable {

// We have to lookup the target(s) of the view since they may need to be underlying into the current context
val newBinding = data.topBinding match {
case ViewBinding(target) => ViewBinding(lookupData(reify(target)))
case avb @ AggregateViewBinding(map) =>
case ViewBinding(target, writable1) =>
val (reified, writable2) = reify(target)
ViewBinding(lookupData(reified), writable1.combine(writable2))
case avb @ AggregateViewBinding(map, _) =>
data match {
case e: Element => ViewBinding(lookupData(reify(avb.lookup(e).get)))
case e: Element =>
val (reified, writable) = reify(avb.lookup(e).get)
ViewBinding(lookupData(reified), avb.lookupWritability(e).combine(writable))
case _: Aggregate =>
// We could just call reifyIdentityView, but since we already have avb, it is
// faster to just use it but then call reifyIdentityView in case the target is itself a view
def reifyOpt(data: Data): Option[Data] = map.get(data).flatMap(reifyIdentityView(_))
def reifyOpt(data: Data): Option[(Data, ViewWriteability)] = map.get(data).flatMap(reifyIdentityView(_))
// Just remap each Data present in the map
val newMap = coiterate(result, data).flatMap {
case (res, from) => reifyOpt(from).map(res -> lookupData(_))
}.toMap
AggregateViewBinding(newMap)
val mapping = coiterate(result, data).flatMap {
case (res, from) => reifyOpt(from).map { case (t, w) => (res, lookupData(t), w) }
}
val newMap = mapping.map { case (from, to, _) => from -> to }.toMap
val wrMap = mapping.flatMap { case (from, _, wr) => Option.when(wr.isReadOnly)(from -> wr) }.toMap
AggregateViewBinding(newMap, Option.when(wrMap.nonEmpty)(wrMap))
}
case _ => throw new InternalErrorException("Match error: data.topBinding=${data.topBinding}")
}
Expand All @@ -197,7 +203,7 @@ object Lookupable {
// We must also mark any non-identity Aggregates as unnammed
newBinding match {
case _: ViewBinding => // Do nothing
case AggregateViewBinding(childMap) =>
case AggregateViewBinding(childMap, _) =>
// TODO we could do reifySingleTarget instead of just marking non-identity mappings
getRecursiveFields.lazily(result, "_").foreach {
case (agg: Aggregate, _) if !childMap.contains(agg) =>
Expand Down
Loading

0 comments on commit 406d936

Please # to comment.