diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 6624bbeac15..992272d3214 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -594,15 +594,88 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { */ private[chisel3] def typeEquivalent(that: Data): Boolean +<<<<<<< HEAD private[chisel3] def requireVisible(): Unit = { +======= + /** Find and report any type mismatches + * + * @param that Data being compared to this + * @param strictTypes Does class of Bundles or Records need to match? Inverse of "structural". + * @param strictWidths do widths need to match? + * @return None if types are equivalent, Some String reporting the first mismatch if not + */ + private[chisel3] final def findFirstTypeMismatch( + that: Data, + strictTypes: Boolean, + strictWidths: Boolean + ): Option[String] = { + def rec(left: Data, right: Data): Option[String] = + (left, right) match { + // Careful, EnumTypes are Element and if we don't implement this, then they are all always equal + case (e1: EnumType, e2: EnumType) => + // TODO, should we implement a form of structural equality for enums? + if (e1.factory == e2.factory) None + else Some(s": Left ($e1) and Right ($e2) have different types.") + // Properties should be considered equal when getPropertyType is equal, not when getClass is equal. + case (p1: Property[_], p2: Property[_]) => + if (p1.getPropertyType != p2.getPropertyType) { + Some(s": Left ($p1) and Right ($p2) have different types") + } else { + None + } + case (e1: Element, e2: Element) if e1.getClass == e2.getClass => + if (strictWidths && e1.width != e2.width) { + Some(s": Left ($e1) and Right ($e2) have different widths.") + } else { + None + } + case (r1: Record, r2: Record) if !strictTypes || r1.getClass == r2.getClass => + val (larger, smaller, msg) = + if (r1._elements.size >= r2._elements.size) (r1, r2, "Left") else (r2, r1, "Right") + larger._elements.collectFirst { + case (name, data) => + val recurse = smaller._elements.get(name) match { + case None => Some(s": Dangling field on $msg") + case Some(data2) => rec(data, data2) + } + recurse.map("." + name + _) + }.flatten + case (v1: Vec[_], v2: Vec[_]) => + if (v1.size != v2.size) { + Some(s": Left (size ${v1.size}) and Right (size ${v2.size}) have different lengths.") + } else { + val recurse = rec(v1.sample_element, v2.sample_element) + recurse.map("[_]" + _) + } + case _ => Some(s": Left ($left) and Right ($right) have different types.") + } + val leftType = if (this.hasBinding) this.cloneType else this + val rightType = if (that.hasBinding) that.cloneType else that + rec(leftType, rightType) + } + + private[chisel3] def isVisible: Boolean = isVisibleFromModule && visibleFromWhen.isEmpty + private[chisel3] def isVisibleFromModule: Boolean = { + val topBindingOpt = this.topBindingOpt // Only call the function once +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) val mod = topBindingOpt.flatMap(_.location) topBindingOpt match { case Some(tb: TopBinding) if (mod == Builder.currentModule) => case Some(pb: PortBinding) +<<<<<<< HEAD if (mod.flatMap(Builder.retrieveParent(_, Builder.currentModule.get)) == Builder.currentModule) => case Some(_: UnconstrainedBinding) => case _ => throwException(s"operand '$this' is not visible from the current module") +======= + 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(pb: SecretPortBinding) => true // Ignore secret to not require visibility + case Some(_: UnconstrainedBinding) => true + case _ => false +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) } MonoConnect.checkWhenVisibility(this) match { case Some(sourceInfo) => diff --git a/core/src/main/scala/chisel3/internal/Binding.scala b/core/src/main/scala/chisel3/internal/Binding.scala index 902227430b4..9b5d833b1a1 100644 --- a/core/src/main/scala/chisel3/internal/Binding.scala +++ b/core/src/main/scala/chisel3/internal/Binding.scala @@ -129,7 +129,13 @@ case class MemTypeBinding[T <: Data](parent: MemBase[T]) extends Binding { case class DontCareBinding() extends UnconstrainedBinding // Views currently only support 1:1 Element-level mappings -private[chisel3] case class ViewBinding(target: Element) extends UnconstrainedBinding +private[chisel3] case class ViewBinding(target: Element) extends Binding with ConditionalDeclarable { + def location: Option[BaseModule] = target.binding.flatMap(_.location) + def visibility: Option[WhenContext] = target.binding.flatMap { + case c: ConditionalDeclarable => c.visibility + case _ => None + } +} /** Binding for Aggregate Views * @param childMap Mapping from children of this view to their respective targets @@ -138,9 +144,26 @@ private[chisel3] case class ViewBinding(target: Element) extends UnconstrainedBi * @note The types of key and value need not match for the top Data in a total view of type * Aggregate */ -private[chisel3] case class AggregateViewBinding(childMap: Map[Data, Data]) extends UnconstrainedBinding { +private[chisel3] case class AggregateViewBinding(childMap: Map[Data, Data]) extends Binding with ConditionalDeclarable { // Helper lookup function since types of Elements always match def lookup(key: Element): Option[Element] = childMap.get(key).map(_.asInstanceOf[Element]) + + // FIXME Technically an AggregateViewBinding can have multiple locations and visibilities + // Fixing this requires an overhaul to this code so for now we just do the best we can + // Return a location if there is a unique one for all targets, None otherwise + lazy val location: Option[BaseModule] = { + val locations = childMap.values.view.flatMap(_.binding.toSeq.flatMap(_.location)).toVector.distinct + if (locations.size == 1) Some(locations.head) + else None + } + lazy val visibility: Option[WhenContext] = { + val contexts = childMap.values.view + .flatMap(_.binding.toSeq.collect { case c: ConditionalDeclarable => c.visibility }.flatten) + .toVector + .distinct + if (contexts.size == 1) Some(contexts.head) + else None + } } /** Binding for Data's returned from accessing an Instance/Definition members, if not readable/writable port */ diff --git a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala index be5a89b5609..ea8abaaecc5 100644 --- a/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala +++ b/src/test/scala/chiselTests/reflect/DataMirrorSpec.scala @@ -6,21 +6,77 @@ import chisel3._ import chisel3.reflect.DataMirror import chiselTests.ChiselFlatSpec import circt.stage.ChiselStage +<<<<<<< HEAD +======= +import chisel3.util.DecoupledIO +import chisel3.experimental.hierarchy._ +import chisel3.experimental.dataview._ +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) object DataMirrorSpec { import org.scalatest.matchers.should.Matchers._ class GrandChild(parent: RawModule) extends Module { DataMirror.getParent(this) should be(Some(parent)) +<<<<<<< HEAD +======= + DataMirror.isVisible(internal) should be(true) + DataMirror.isVisible(internal.viewAs[Bool]) should be(true) +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) } + @instantiable class Child(parent: RawModule) extends Module { val inst = Module(new GrandChild(this)) +<<<<<<< HEAD DataMirror.getParent(inst) should be(Some(this)) DataMirror.getParent(this) should be(Some(parent)) +======= + @public val io = IO(Input(Bool())) + val internal = WireInit(false.B) + lazy val underWhen = WireInit(false.B) + when(true.B) { + underWhen := true.B // trigger the lazy val + DataMirror.isVisible(underWhen) should be(true) + DataMirror.isVisible((internal, underWhen).viewAs) should be(true) + } + val mixedView = (io, underWhen).viewAs + DataMirror.getParent(inst) should be(Some(this)) + DataMirror.getParent(this) should be(Some(parent)) + DataMirror.isVisible(io) should be(true) + DataMirror.isVisible(io.viewAs[Bool]) should be(true) + DataMirror.isVisible(internal) should be(true) + DataMirror.isVisible(internal.viewAs[Bool]) should be(true) + DataMirror.isVisible(inst.internal) should be(false) + DataMirror.isVisible(inst.internal.viewAs[Bool]) should be(false) + DataMirror.isVisible(underWhen) should be(false) + DataMirror.isVisible(underWhen.viewAs) should be(false) + DataMirror.isVisible(mixedView) should be(false) + DataMirror.isVisible(mixedView._1) should be(true) +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) } + @instantiable class Parent extends Module { +<<<<<<< HEAD val inst = Module(new Child(this)) DataMirror.getParent(inst) should be(Some(this)) DataMirror.getParent(this) should be(None) +======= + @public val io = IO(Input(Bool())) + @public val inst = Module(new Child(this)) + @public val internal = WireInit(io) + @public val tuple = (io, internal).viewAs + inst.io := internal + DataMirror.getParent(inst) should be(Some(this)) + DataMirror.getParent(this) should be(None) + DataMirror.isVisible(inst.io) should be(true) + DataMirror.isVisible(inst.io.viewAs[Bool]) should be(true) + DataMirror.isVisible(inst.internal) should be(false) + DataMirror.isVisible(inst.internal.viewAs[Bool]) should be(false) + DataMirror.isVisible(inst.inst.internal) should be(false) + DataMirror.isVisible(inst.inst.internal.viewAs[Bool]) should be(false) + DataMirror.isVisible(inst.mixedView) should be(false) + DataMirror.isVisible(inst.mixedView._1) should be(true) + DataMirror.isVisible(tuple) should be(true) +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) } } @@ -74,16 +130,28 @@ class DataMirrorSpec extends ChiselFlatSpec { ChiselStage.elaborate(new MyModule) } +<<<<<<< HEAD it should "support getParent for normal modules" in { ChiselStage.elaborate(new Parent) +======= + it should "support getParent and isVisible for normal modules" in { + ChiselStage.emitCHIRRTL(new Parent) +>>>>>>> 84a21f8a7 (Fix visibility for views (#3818)) } - it should "support getParent for normal modules even when used in a D/I context" in { - import chisel3.experimental.hierarchy._ + it should "support getParent and isVisible for normal modules even when used in a D/I context" in { class Top extends Module { val defn = Definition(new Parent) val inst = Instance(defn) DataMirror.getParent(this) should be(None) + DataMirror.isVisible(inst.io) should be(true) + DataMirror.isVisible(inst.io.viewAs) should be(true) + DataMirror.isVisible(inst.inst.io) should be(false) + DataMirror.isVisible(inst.inst.io.viewAs) should be(false) + DataMirror.isVisible(inst.internal) should be(false) + DataMirror.isVisible(inst.internal.viewAs) should be(false) + DataMirror.isVisible(inst.tuple) should be(false) + DataMirror.isVisible(inst.tuple._1) should be(true) } ChiselStage.elaborate(new Top) }