Skip to content

Commit

Permalink
Add support for marking things as readOnly (backport #4190) (#4194)
Browse files Browse the repository at this point in the history 
* Add support for marking things as readOnly (#4190)

Also add internal support for marking things as "deprecated read only".

(cherry picked from commit 7be4243)

# Conflicts:
#	core/src/main/scala/chisel3/experimental/hierarchy/core/Lookupable.scala
#	core/src/main/scala/chisel3/internal/Binding.scala

* Resolve backport conflicts

---------

Co-authored-by: Jack Koenig <koenig@sifive.com>
  • Loading branch information
@mergify @jackkoenig
mergify[bot] and jackkoenig authored Jun 20, 2024
1 parent 3c7b017 commit c9957f9
Show file tree
Hide file tree
Showing 14 changed files with 730 additions and 144 deletions.
4 changes: 2 additions & 2 deletions core/src/main/scala/chisel3/Aggregate.scala
View file
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
View file
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
View file
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
View file
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
34 changes: 24 additions & 10 deletions core/src/main/scala/chisel3/experimental/hierarchy/core/Lookupable.scala
View file
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

0 comments on commit c9957f9

Please sign in to comment.