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

Add support for marking things as readOnly (backport #4190) #4194

Merged
merged 2 commits into from
Jun 20, 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
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 @@ -47,7 +47,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 @@ -354,7 +354,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
29 changes: 22 additions & 7 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.sourceinfo._
Expand Down Expand Up @@ -628,8 +628,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 @@ -650,13 +650,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 @@ -676,11 +679,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 @@ -998,6 +1001,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 @@ -38,9 +38,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 @@ -50,7 +53,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 @@ -19,15 +19,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 @@ -37,6 +43,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 @@ -74,9 +102,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 @@ -204,7 +233,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 @@ -221,15 +250,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 @@ -246,19 +279,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 @@ -268,19 +307,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 @@ -297,15 +341,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 @@ -10,7 +10,15 @@ import scala.collection.mutable.HashMap
import chisel3._
import chisel3.experimental.dataview.{isView, reify, reifyIdentityView}
import chisel3.internal.firrtl.ir.{Arg, ILit, Index, ModuleIO, Slot, ULit}
import chisel3.internal.{throwException, AggregateViewBinding, Builder, ChildBinding, ViewBinding, ViewParent}
import chisel3.internal.{
throwException,
AggregateViewBinding,
Builder,
ChildBinding,
ViewBinding,
ViewParent,
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 @@ -175,19 +183,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 @@ -196,7 +210,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
Loading