diplomacy: place Monitors inside LazyModules sinks

We used to place Monitors at the point of the ':='.
This was problematic because the clock domain might be wrong.
Thus, we needed to shove Monitors a lot.

Furthermore, now that we have cross-module ':=', you might not even
have access to the wires at the point where ':=' is invoked.

src/main/scala/diplomacy/Monitor.scala

@@ -1,10 +0,0 @@
-// See LICENSE.SiFive for license details.
-
-package freechips.rocketchip.diplomacy
-
-import chisel3.internal.sourceinfo.{SourceInfo, SourceLine}
-import freechips.rocketchip.config.Parameters
-
-abstract class MonitorBase(implicit val sourceInfo: SourceInfo, p: Parameters) extends LazyModule()(p) {
-  override val module: LazyModuleImp
-}

src/main/scala/diplomacy/Nodes.scala

@@ -36,12 +36,9 @@ trait InwardNodeImp[DI, UI, EI, BI <: Data]
 {
   def edgeI(pd: DI, pu: UI, p: Parameters, sourceInfo: SourceInfo): EI
   def bundleI(ei: EI): BI
+  def monitor(bundle: BI, edge: EI) {}
   def colour: String
   def reverse: Boolean = false
-  def connect(edges: () => Seq[EI], bundles: () => Seq[(BI, BI)], enableMonitoring: Boolean)
-              (implicit p: Parameters, sourceInfo: SourceInfo): (Option[MonitorBase], () => Unit) = {
-    (None, () => bundles().foreach { case (i, o) => i <> o })
-  }
 
   // optional methods to track node graph
   def mixI(pu: UI, node: InwardNode[DI, UI, BI]): UI = pu // insert node into parameters
@@ -104,14 +101,10 @@ case class NodeHandle[DI, UI, BI <: Data, DO, UO, BO <: Data]
 trait InwardNodeHandle[DI, UI, BI <: Data]
 {
   protected[diplomacy] val inward: InwardNode[DI, UI, BI]
-  def := (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] =
+  def := (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) { inward.:=(h)(p, sourceInfo) }
-    inward.:=(h)(p, sourceInfo)
+  def :*= (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) { inward.:*=(h)(p, sourceInfo) }
-  def :*= (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] =
+  def :=* (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) { inward.:=*(h)(p, sourceInfo) }
-    inward.:*=(h)(p, sourceInfo)
+  def :=? (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) { inward.:=?(h)(p, sourceInfo) }
-  def :=* (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] =
-    inward.:=*(h)(p, sourceInfo)
-  def :=? (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] =
-    inward.:=?(h)(p, sourceInfo)
 }
 
 sealed trait NodeBinding
@@ -267,13 +260,7 @@ sealed abstract class MixedNode[DI, UI, EI, BI <: Data, DO, UO, EO, BO <: Data](
       data   = bundleIn(i))
   }
 
-  // Used by LazyModules.module.instantiate
   private var bundlesSafeNow = false
-  protected[diplomacy] def instantiate() = {
-    bundlesSafeNow = true
-    danglesOut ++ danglesIn
-  }
-
   // Accessors to the result of negotiation to be used in LazyModuleImp:
   def out: Seq[(BO, EO)] = {
     require(bundlesSafeNow, s"${name}.out should only be called from the context of its module implementation")
@@ -284,9 +271,19 @@ sealed abstract class MixedNode[DI, UI, EI, BI <: Data, DO, UO, EO, BO <: Data](
     bundleIn zip edgesIn
   }
 
+  // Used by LazyModules.module.instantiate
+  protected val identity = false
+  protected[diplomacy] def instantiate() = {
+    bundlesSafeNow = true
+    if (!identity) {
+      (iPorts zip in) foreach {
+        case ((_, _, p, _), (b, e)) => if (p(MonitorsEnabled)) inner.monitor(b, e)
+    } }
+    danglesOut ++ danglesIn
+  }
+
   // connects the outward part of a node with the inward part of this node
-  private def bind(h: OutwardNodeHandle[DI, UI, BI], binding: NodeBinding)
+  private def bind(h: OutwardNodeHandle[DI, UI, BI], binding: NodeBinding)(implicit p: Parameters, sourceInfo: SourceInfo) {
-                  (implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] = {
     val x = this // x := y
     val y = h.outward
     val info = sourceLine(sourceInfo, " at ", "")
@@ -298,13 +295,12 @@ sealed abstract class MixedNode[DI, UI, EI, BI <: Data, DO, UO, EO, BO <: Data](
       case BIND_STAR  => BIND_QUERY
       case BIND_QUERY => BIND_STAR })
     x.iPush(o, y, binding)
-    None // !!! create monitors
   }
 
-  override def :=  (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] = bind(h, BIND_ONCE)
+  override def :=  (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) = bind(h, BIND_ONCE)
-  override def :*= (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] = bind(h, BIND_STAR)
+  override def :*= (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) = bind(h, BIND_STAR)
-  override def :=* (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] = bind(h, BIND_QUERY)
+  override def :=* (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) = bind(h, BIND_QUERY)
-  override def :=? (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo): Option[MonitorBase] = {
+  override def :=? (h: OutwardNodeHandle[DI, UI, BI])(implicit p: Parameters, sourceInfo: SourceInfo) = {
     p(CardinalityInferenceDirectionKey) match {
       case CardinalityInferenceDirection.SOURCE_TO_SINK => this :=* h
       case CardinalityInferenceDirection.SINK_TO_SOURCE => this :*= h
@@ -378,6 +374,7 @@ class AdapterNode[D, U, EO, EI, B <: Data](imp: NodeImp[D, U, EO, EI, B])(
 class IdentityNode[D, U, EO, EI, B <: Data](imp: NodeImp[D, U, EO, EI, B])()(implicit valName: ValName)
   extends AdapterNode(imp)({ s => s }, { s => s })
 {
+  protected override val identity = true
   override protected[diplomacy] def instantiate() = {
     val dangles = super.instantiate()
     (out zip in) map { case ((o, _), (i, _)) => o <> i }

src/main/scala/tilelink/AsyncCrossing.scala

@@ -94,9 +94,9 @@ class TLAsyncCrossing(depth: Int = 8, sync: Int = 3)(implicit p: Parameters) ext
   val source = LazyModule(new TLAsyncCrossingSource(sync))
   val sink = LazyModule(new TLAsyncCrossingSink(depth, sync))
 
-  val _    = (sink.node := source.node) // no monitor
+  sink.node := source.node
-  val in   = (source.node := nodeIn)
+  source.node := nodeIn
-  val out  = (nodeOut := sink.node)
+  nodeOut := sink.node
 
   lazy val module = new LazyModuleImp(this) {
     val io = IO(new Bundle {
@@ -108,17 +108,8 @@ class TLAsyncCrossing(depth: Int = 8, sync: Int = 3)(implicit p: Parameters) ext
 
     source.module.clock := io.in_clock
     source.module.reset := io.in_reset
-    in.foreach { lm =>
-      lm.module.clock := io.in_clock
-      lm.module.reset := io.in_reset
-    }
-
     sink.module.clock := io.out_clock
     sink.module.reset := io.out_reset
-    out.foreach { lm =>
-      lm.module.clock := io.out_clock
-      lm.module.reset := io.out_reset
-    }
   }
 }
 
@@ -133,7 +124,7 @@ class TLRAMAsyncCrossing(txns: Int)(implicit p: Parameters) extends LazyModule {
 
   model.node := fuzz.node
   cross.node := TLFragmenter(4, 256)(TLDelayer(0.1)(model.node))
-  val monitor = (ram.node := cross.node)
+  ram.node := cross.node
 
   lazy val module = new LazyModuleImp(this) with UnitTestModule {
     io.finished := fuzz.module.io.finished
@@ -147,12 +138,6 @@ class TLRAMAsyncCrossing(txns: Int)(implicit p: Parameters) extends LazyModule {
     cross.module.io.in_reset := reset
     cross.module.io.out_clock := clocks.io.clock_out
     cross.module.io.out_reset := reset
-
-    // Push the Monitor into the right clock domain
-    monitor.foreach { m =>
-      m.module.clock := clocks.io.clock_out
-      m.module.reset := reset
-    }
   }
 }
 

src/main/scala/tilelink/Fuzzer.scala

@@ -240,7 +240,7 @@ class TLFuzzRAM(txns: Int)(implicit p: Parameters) extends LazyModule
   ram2.node := TLFragmenter(16, 256)(xbar2.node)
   xbar.node := TLWidthWidget(16)(TLHintHandler()(xbar2.node))
   cross.node := TLFragmenter(4, 256)(TLBuffer()(xbar.node))
-  val monitor = (ram.node := cross.node)
+  ram.node := cross.node
   gpio.node := TLFragmenter(4, 32)(TLBuffer()(xbar.node))
 
   lazy val module = new LazyModuleImp(this) with UnitTestModule {
@@ -255,12 +255,6 @@ class TLFuzzRAM(txns: Int)(implicit p: Parameters) extends LazyModule
     cross.module.io.in_reset := reset
     cross.module.io.out_clock := clocks.io.clock_out
     cross.module.io.out_reset := reset
-
-    // Push the Monitor into the right clock domain
-    monitor.foreach { m =>
-      m.module.clock := clocks.io.clock_out
-      m.module.reset := reset
-    }
   }
 }
 

src/main/scala/tilelink/Monitor.scala

@@ -8,26 +8,22 @@ import freechips.rocketchip.config.Parameters
 import freechips.rocketchip.diplomacy._
 import freechips.rocketchip.util.{HeterogeneousBag, PlusArg}
 
-case class TLMonitorArgs(edge: () => Seq[TLEdge], sourceInfo: SourceInfo, p: Parameters)
+case class TLMonitorArgs(edge: TLEdge)
 
-abstract class TLMonitorBase(args: TLMonitorArgs) extends MonitorBase()(args.sourceInfo, args.p)
+abstract class TLMonitorBase(args: TLMonitorArgs) extends Module
 {
-  def legalize(bundle: TLBundleSnoop, edge: TLEdge, reset: Bool): Unit
+  val io = new Bundle {
-
+    val in = new TLBundleSnoop(args.edge.bundle).flip
-  lazy val module = new LazyModuleImp(this) {
-    val edges = args.edge()
-    val io = IO(new Bundle {
-      val in = HeterogeneousBag(edges.map(p => new TLBundleSnoop(p.bundle))).flip
-    })
-
-    (edges zip io.in).foreach { case (e, in) => legalize(in, e, reset) }
   }
+
+  def legalize(bundle: TLBundleSnoop, edge: TLEdge, reset: Bool): Unit
+  legalize(io.in, args.edge, reset)
 }
 
 class TLMonitor(args: TLMonitorArgs) extends TLMonitorBase(args)
 {
   def extra = {
-    sourceInfo match {
+    args.edge.sourceInfo match {
       case SourceLine(filename, line, col) => s" (connected at $filename:$line:$col)"
       case _ => ""
     }

src/main/scala/tilelink/Nodes.scala

@@ -10,7 +10,6 @@ import freechips.rocketchip.util.RationalDirection
 import scala.collection.mutable.ListBuffer
 
 case object TLMonitorBuilder extends Field[TLMonitorArgs => TLMonitorBase](args => new TLMonitor(args))
-case object TLCombinationalCheck extends Field[Boolean](false)
 
 object TLImp extends NodeImp[TLClientPortParameters, TLManagerPortParameters, TLEdgeOut, TLEdgeIn, TLBundle]
 {
@@ -24,25 +23,9 @@ object TLImp extends NodeImp[TLClientPortParameters, TLManagerPortParameters, TL
   override def labelI(ei: TLEdgeIn)  = (ei.manager.beatBytes * 8).toString
   override def labelO(eo: TLEdgeOut) = (eo.manager.beatBytes * 8).toString
 
-  override def connect(edges: () => Seq[TLEdgeIn], bundles: () => Seq[(TLBundle, TLBundle)], enableMonitoring: Boolean)
+  override def monitor(bundle: TLBundle, edge: TLEdgeIn) {
-                      (implicit p: Parameters, sourceInfo: SourceInfo): (Option[TLMonitorBase], () => Unit) = {
+    val monitor = Module(edge.params(TLMonitorBuilder)(TLMonitorArgs(edge)))
-    val monitor = if (enableMonitoring) Some(LazyModule(p(TLMonitorBuilder)(TLMonitorArgs(edges, sourceInfo, p)))) else None
+    monitor.io.in := TLBundleSnoop(bundle, bundle)
-    (monitor, () => {
-      val eval = bundles ()
-      monitor.foreach { m => (eval zip m.module.io.in) foreach { case ((i,o), m) => m := TLBundleSnoop(o,i) } }
-      eval.foreach { case (bi, bo) =>
-        bi <> bo
-        if (p(TLCombinationalCheck)) {
-          // It is forbidden for valid to depend on ready in TL2
-          // If someone did that, then this will create a detectable combinational loop
-          bo.a.ready := bi.a.ready && bo.a.valid
-          bi.b.ready := bo.b.ready && bi.b.valid
-          bo.c.ready := bi.c.ready && bo.c.valid
-          bi.d.ready := bo.d.ready && bi.d.valid
-          bo.e.ready := bi.e.ready && bo.e.valid
-        }
-      }
-    })
   }
 
   override def mixO(pd: TLClientPortParameters, node: OutwardNode[TLClientPortParameters, TLManagerPortParameters, TLBundle]): TLClientPortParameters  =

src/main/scala/tilelink/RationalCrossing.scala

@@ -106,9 +106,9 @@ class TLRationalCrossing(direction: RationalDirection = Symmetric)(implicit p: P
   val source = LazyModule(new TLRationalCrossingSource)
   val sink = LazyModule(new TLRationalCrossingSink(direction))
 
-  val _    = (sink.node := source.node) // no monitor
+  sink.node := source.node
-  val in   = (source.node := nodeIn)
+  source.node := nodeIn
-  val out  = (nodeOut := sink.node)
+  nodeOut := sink.node
 
   lazy val module = new LazyModuleImp(this) {
     val io = IO(new Bundle {
@@ -120,17 +120,8 @@ class TLRationalCrossing(direction: RationalDirection = Symmetric)(implicit p: P
 
     source.module.clock := io.in_clock
     source.module.reset := io.in_reset
-    in.foreach { lm =>
-      lm.module.clock := io.in_clock
-      lm.module.reset := io.in_reset
-    }
-
     sink.module.clock := io.out_clock
     sink.module.reset := io.out_reset
-    out.foreach { lm =>
-      lm.module.clock := io.out_clock
-      lm.module.reset := io.out_reset
-    }
   }
 }