FixedIO__Modules with various kinds of probe ports (#4105)

* - add probe IOs to FixedIOModuleSpec * getMatchedFields: don't recurse through probes. * update scaladoc for FlatIO for probe types --------- Co-authored-by: Will Dietz <will.dietz@sifive.com> (cherry picked from commit 0553b69) # Conflicts: # core/src/main/scala/chisel3/Data.scala
chipsalliance · May 30, 2024 · 7971782 · 7971782
1 parent c6eff45
commit 7971782
Show file tree

Hide file tree

Showing 5 changed files with 223 additions and 13 deletions.
diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
@@ -214,8 +214,12 @@ private[chisel3] object getRecursiveFields {
 private[chisel3] object getMatchedFields {
   def apply(x: Data, y: Data): Seq[(Data, Data)] = (x, y) match {
     case (x: Element, y: Element) =>
-      require(x.typeEquivalent(y))
+      x.requireTypeEquivalent(y)
       Seq(x -> y)
+    case (_, _) if DataMirror.hasProbeTypeModifier(x) || DataMirror.hasProbeTypeModifier(y) => {
+      x.requireTypeEquivalent(y)
+      Seq(x -> y)
+    }
     case (x: Record, y: Record) =>
       (x._elements
         .zip(y._elements))
@@ -577,9 +581,29 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
           }
         case _ => Some(s": Left ($left) and Right ($right) have different types.")
       }
+<<<<<<< HEAD
     val leftType = if (this.hasBinding) this.cloneType else this
     val rightType = if (that.hasBinding) that.cloneType else that
     rec(leftType, rightType)
+=======
+
+    rec(this, that)
+  }
+
+  /** Require that two things are type equivalent, and if they are not, print a helpful error message as
+    * to why not.
+    */
+  private[chisel3] def requireTypeEquivalent(that: Data): Unit = {
+    require(
+      this.typeEquivalent(that), {
+        val reason = this
+          .findFirstTypeMismatch(that, strictTypes = true, strictWidths = true, strictProbeInfo = true)
+          .map(s => s"\nbecause $s")
+          .getOrElse("")
+        s"$this is not typeEquivalent to $that$reason"
+      }
+    )
+>>>>>>> 0553b6943 (FixedIO__Modules with various kinds of probe ports  (#4105))
   }
 
   private[chisel3] def isVisible: Boolean = isVisibleFromModule && visibleFromWhen.isEmpty

diff --git a/core/src/main/scala/chisel3/IO.scala b/core/src/main/scala/chisel3/IO.scala
@@ -4,10 +4,11 @@ import chisel3.internal.{throwException, Builder}
 import chisel3.experimental.{noPrefix, requireIsChiselType, SourceInfo}
 import chisel3.properties.{Class, Property}
 import chisel3.reflect.DataMirror.internal.chiselTypeClone
+import chisel3.reflect.DataMirror.{hasProbeTypeModifier, specifiedDirectionOf}
 
 object IO {
 
-  /** Constructs a port for the current Module
+  /** Constructs a port for the current Module.
     *
     * This must wrap the datatype used to set the io field of any Module.
     * i.e. All concrete modules must have defined io in this form:
@@ -19,7 +20,6 @@ object IO {
     *
     * Also registers a Data as a port, also performing bindings. Cannot be called once ports are
     * requested (so that all calls to ports will return the same information).
-    * Internal API.
     */
   def apply[T <: Data](iodef: => T)(implicit sourceInfo: SourceInfo): T = {
     val module = Module.currentModule.get // Impossible to fail
@@ -65,7 +65,7 @@ object IO {
 
 /** The same as [[IO]] except there is no prefix when given a [[Record]] or
   * [[Bundle]].  For [[Element]] ([[UInt]], etc.) or [[Vec]] types, this is
-  * the same as [[IO]].
+  * the same as [[IO]]. It is also the same as [[IO]] for [[chisel3.probe.Probe]] types.
   *
   * @example {{{
   * class MyBundle extends Bundle {
@@ -82,9 +82,10 @@ object IO {
 object FlatIO {
   def apply[T <: Data](gen: => T)(implicit sourceInfo: SourceInfo): T = noPrefix {
     import chisel3.experimental.dataview._
-    def coerceDirection(d: Data) = {
+
+    def coerceDirection(d: Data): Data = {
       import chisel3.{SpecifiedDirection => SD}
-      chisel3.reflect.DataMirror.specifiedDirectionOf(gen) match {
+      specifiedDirectionOf(gen) match {
         case SD.Flip   => Flipped(d)
         case SD.Input  => Input(d)
         case SD.Output => Output(d)
@@ -94,13 +95,14 @@ object FlatIO {
 
     type R = T with Record
     gen match {
+      case d if hasProbeTypeModifier(d) => IO(d)
       case _:      Element => IO(gen)
       case _:      Vec[_] => IO(gen)
       case record: R =>
         val ports: Seq[Data] =
           record._elements.toSeq.reverse.map {
             case (name, data) =>
-              val p = chisel3.IO(coerceDirection(chiselTypeClone(data).asInstanceOf[Data]))
+              val p = IO(coerceDirection(data).asInstanceOf[Data])
               p.suggestName(name)
               p
 

diff --git a/core/src/main/scala/chisel3/experimental/dataview/package.scala b/core/src/main/scala/chisel3/experimental/dataview/package.scala
@@ -154,7 +154,13 @@ package object dataview {
       case (aa: Aggregate, ba: Aggregate) =>
         if (!ba.typeEquivalent(aa)) {
           val fieldName = viewFieldLookup(ba)
-          throw InvalidViewException(s"field $fieldName specified as view of non-type-equivalent value $aa")
+          val reason = ba
+            .findFirstTypeMismatch(aa, strictTypes = true, strictWidths = true, strictProbeInfo = true)
+            .map(s => s"\nbecause $s")
+            .getOrElse("")
+          throw InvalidViewException(
+            s"Field $fieldName specified as view of non-type-equivalent value $aa$reason"
+          )
         }
         getMatchedFields(aa, ba).foreach {
           case (aelt: Element, belt: Element) => onElt(aelt, belt)

diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala
@@ -9,7 +9,7 @@ import chisel3.internal.containsProbe
 import chisel3.internal.Builder.pushCommand
 import chisel3.internal.firrtl.ir.{Connect, DefInvalid, ProbeDefine, PropAssign}
 import chisel3.internal.firrtl.Converter
-import chisel3.experimental.dataview.{isView, reify, reifyToAggregate}
+import chisel3.experimental.dataview.{isView, reify, reifySingleData, reifyToAggregate}
 import chisel3.properties.{Class, Property}
 import chisel3.reflect.DataMirror
 
@@ -439,11 +439,22 @@ private[chisel3] object MonoConnect {
   }
 
   def probeDefine(
-    sourceInfo: SourceInfo,
-    sink:       Data,
-    source:     Data,
-    context:    BaseModule
+    sourceInfo:  SourceInfo,
+    sinkProbe:   Data,
+    sourceProbe: Data,
+    context:     BaseModule
   ): Unit = {
+
+    val sink = reifySingleData(sinkProbe).getOrElse(
+      throwException(
+        s"If a DataView contains a Probe, it must resolve to one Data. $sinkProbe does not meet this criteria."
+      )
+    )
+    val source = reifySingleData(sourceProbe).getOrElse(
+      throwException(
+        s"If a DataView contains a Probe, it must resolve to one Data. $sourceProbe does not meet this criteria."
+      )
+    )
     checkConnect.checkConnection(sourceInfo, sink, source, context)
     context match {
       case rm: RawModule => rm.addCommand(ProbeDefine(sourceInfo, sink.lref, source.ref))

diff --git a/src/test/scala/chiselTests/FixedIOModuleSpec.scala b/src/test/scala/chiselTests/FixedIOModuleSpec.scala
@@ -4,6 +4,10 @@ package chiselTests
 
 import chisel3._
 import circt.stage.ChiselStage
+import scala.collection.immutable.ListMap
+import chisel3.reflect.DataMirror.internal.chiselTypeClone
+import chisel3.experimental.SourceInfo
+import chisel3.probe.Probe
 
 class FixedIOModuleSpec extends ChiselFlatSpec with Utils with MatchesAndOmits {
 
@@ -88,4 +92,167 @@ class FixedIOModuleSpec extends ChiselFlatSpec with Utils with MatchesAndOmits {
       "input end : UInt<1>"
     )()
   }
+
+  class Agg extends Bundle {
+    val foo = Bool()
+    val bar = Bool()
+  }
+
+  class AggWithProbes extends Bundle {
+    val foo = Probe(Bool())
+    val bar = Probe(Bool())
+  }
+
+  class MixedBundle(probeAgg: Boolean, aggProbe: Boolean) extends Bundle {
+    val elem = Probe(Bool())
+    val agg = Option.when(probeAgg)(Probe(new Agg()))
+    val nested = Option.when(aggProbe)(new AggWithProbes())
+  }
+
+  class ExampleRaw(probeAgg: Boolean, aggProbe: Boolean)
+      extends FixedIORawModule[MixedBundle](new MixedBundle(probeAgg, aggProbe)) {
+
+    val elemWire = Wire(Bool())
+    elemWire := false.B
+    probe.define(io.elem, probe.ProbeValue(elemWire))
+
+    if (probeAgg) {
+      val aggWire = Wire(new Agg())
+      aggWire := DontCare
+      probe.define(io.agg.get, probe.ProbeValue(aggWire))
+    }
+
+    if (aggProbe) {
+      val nestedWire = Wire(new AggWithProbes())
+      val nestedFoo = WireInit(false.B)
+      val nestedBar = WireInit(false.B)
+      probe.define(nestedWire.foo, probe.ProbeValue(nestedFoo))
+      probe.define(nestedWire.bar, probe.ProbeValue(nestedBar))
+      io.nested.get :<>= nestedWire
+    }
+  }
+
+  class ExampleExt(probeAgg: Boolean, aggProbe: Boolean)
+      extends FixedIOExtModule[MixedBundle](new MixedBundle(probeAgg, aggProbe))
+
+  class Parent(probeAgg: Boolean, aggProbe: Boolean) extends Module {
+    val childRaw = Module(new ExampleRaw(probeAgg, aggProbe))
+    val childExt = Module(new ExampleExt(probeAgg, aggProbe))
+
+    // Check Probe(Element)
+    val outElemRaw = IO(Bool())
+    val probeElemWireRaw = Wire(Probe(Bool()))
+    outElemRaw := probe.read(probeElemWireRaw)
+    probeElemWireRaw :<>= childRaw.io.elem
+
+    val probeElemWireExt = Wire(Probe(Bool()))
+    val outElemExt = IO(Bool())
+    outElemExt := probe.read(probeElemWireExt)
+    probeElemWireExt :<>= childExt.io.elem
+
+    // Check Probe(Aggregate)
+    if (probeAgg) {
+      val outAggRaw = IO(new Agg())
+      val probeAggWireRaw = Wire(Probe(new Agg()))
+      outAggRaw := probe.read(probeAggWireRaw)
+      probeAggWireRaw :<>= childRaw.io.agg.get
+      val probeAggWireExt = Wire(Probe(new Agg()))
+      val outAggExt = IO(new Agg())
+      outAggExt := probe.read(probeAggWireExt)
+      probeAggWireExt :<>= childExt.io.agg.get
+    }
+
+    // Check Aggregate(Probe)
+    if (aggProbe) {
+      val probeNestedWireRaw = Wire(new AggWithProbes())
+      val outNestedRawFoo = IO(Bool())
+      val outNestedRawBar = IO(Bool())
+      outNestedRawFoo := probe.read(probeNestedWireRaw.foo)
+      outNestedRawBar := probe.read(probeNestedWireRaw.bar)
+      probeNestedWireRaw :<>= childRaw.io.nested.get
+
+      val probeNestedWireExt = Wire(new AggWithProbes())
+      val outNestedExtFoo = IO(Bool())
+      val outNestedExtBar = IO(Bool())
+      outNestedExtFoo := probe.read(probeNestedWireExt.foo)
+      outNestedExtBar := probe.read(probeNestedWireExt.bar)
+      probeNestedWireExt :<>= childExt.io.nested.get
+    }
+
+  }
+
+  "FixedIORaw/ExtModules" should "be able to have a Record with Probes of Elements in their IOs" in {
+    matchesAndOmits(ChiselStage.emitCHIRRTL(new Parent(false, false)))(
+      "output elem : Probe<UInt<1>>",
+      "output elem : Probe<UInt<1>>",
+      "define probeElemWireRaw = childRaw.elem",
+      "define probeElemWireExt = childExt.elem"
+    )()
+  }
+
+  "FixedIORaw/ExtModules" should "be able to have a Record with Probes of Aggregates in their IOs" in {
+    matchesAndOmits(ChiselStage.emitCHIRRTL(new Parent(true, false)))(
+      "output agg : Probe<{ foo : UInt<1>, bar : UInt<1>}>",
+      "output agg : Probe<{ foo : UInt<1>, bar : UInt<1>}>",
+      "define probeAggWireRaw = childRaw.agg",
+      "define probeAggWireExt = childExt.agg"
+    )()
+  }
+
+  "FixedIORaw/ExtModules" should "be able to have a Record with Aggregates with Probes in their IOs" in {
+    matchesAndOmits(ChiselStage.emitCHIRRTL(new Parent(false, true)))(
+      "output nested : { foo : Probe<UInt<1>>, bar : Probe<UInt<1>>}",
+      "output nested : { foo : Probe<UInt<1>>, bar : Probe<UInt<1>>}",
+      "define probeNestedWireRaw.bar = childRaw.nested.bar",
+      "define probeNestedWireRaw.foo = childRaw.nested.foo",
+      "define probeNestedWireExt.bar = childExt.nested.bar",
+      "define probeNestedWireExt.foo = childExt.nested.foo"
+    )()
+  }
+  "FixedIOExtModules" should "be able to have a Probe(Element) as its FixedIO" in {
+    class ProbeElemExt extends FixedIOExtModule(Probe(Bool()))
+    class Parent extends RawModule {
+      val child = Module(new ProbeElemExt)
+      val ioElem = IO(Output(Bool()))
+      val wireElem = Wire(Probe(Bool()))
+      wireElem :<>= child.io
+      ioElem := probe.read(wireElem)
+    }
+    matchesAndOmits(ChiselStage.emitCHIRRTL(new Parent()))(
+      "output io : Probe<UInt<1>>",
+      "define wireElem = child.io"
+    )()
+  }
+  "FixedIOExtModules" should "be able to have a Probe(Aggregate) as its FixedIO" in {
+    class ProbeAggExt extends FixedIOExtModule(Probe(new Agg()))
+    class Parent extends RawModule {
+      val child = Module(new ProbeAggExt)
+      val ioAgg = IO(Output(new Agg()))
+      val wireAgg = Wire(Probe(new Agg()))
+      wireAgg :<>= child.io
+      ioAgg := probe.read(wireAgg)
+    }
+    matchesAndOmits(ChiselStage.emitCHIRRTL(new Parent()))(
+      "output io : Probe<{ foo : UInt<1>, bar : UInt<1>}>",
+      "define wireAgg = child.io"
+    )()
+  }
+
+  "FixedIOExtModules" should "be able to have an Aggregate with Probes as its FixedIO" in {
+    class ProbeNestedExt extends FixedIOExtModule(new AggWithProbes())
+    class Parent extends RawModule {
+      val child = Module(new ProbeNestedExt)
+      val ioBar = IO(Output(new Bool()))
+      val wireNested = Wire(new AggWithProbes())
+      wireNested :<>= child.io
+      ioBar := probe.read(wireNested.bar)
+    }
+    matchesAndOmits(ChiselStage.emitCHIRRTL(new Parent()))(
+      "output foo : Probe<UInt<1>>",
+      "output bar : Probe<UInt<1>>",
+      "define wireNested.bar = child.bar",
+      "define wireNested.foo = child.foo"
+    )()
+  }
+
 }