『Help』Component Interface Name Changes Result in SpinalHDL Generating Excessive Redundant Module.v Files.

[dreamflyings]

Hello! While writing multi-clock domain code, I discovered that referencing clock domains can cause changes in the interface names of identical components. SpinalHDL mistakenly treats these components as different modules due to the interface name variations, resulting in an inability to reuse code. To address this issue, I extracted common elements and made the following modifications to the demo code (including changes to the PhasePullClockDomains object in the Phase.scala file) to improve code reusability. However, I feel that this modification lacks elegance and may hide bugs. Therefore, I’m raising this common issue to explore a more permanent solution. ⸜(๑'ᵕ'๑)⸝⋆*Below is my code:

import spinal.core._
import spinal.lib._
object SimpleStreamFifoCC{
 def apply[T <: Data](sin: Stream[T], pushClock: ClockDomain, popClock: ClockDomain, withAsync:Boolean = true): Stream[T] = {
 val cdA = withAsync generate new AreaRoot {
 val sq = new SimpleStreamFifoCC(sin.payloadType, 2)
 pushClock.setSynchronousWith(sq.pushCD)
 popClock.setSynchronousWith(sq.popCD)
 sq.io.sin_clk := pushClock.readClockWire
 sq.io.sin_resetn := pushClock.readResetWire
 sq.io.sout_clk := popClock.readClockWire
 sq.io.sout_resetn := popClock.readResetWire
 sq.io.sin << sin
 }
 withAsync.mux(cdA.sq.io.sout, sin)
 }
}
class SimpleStreamFifoCC[T<:Data](DType:HardType[T], size: Int) extends Module {
 val io = new Bundle{
 val sin_clk = in Bool()
 val sin_resetn = in Bool()
 val sout_clk = in Bool()
 val sout_resetn = in Bool()
 val sin = slave Stream(DType())
 val sout = master Stream(DType())
 }
 val pushCD = ClockDomain(clock = io.sin_clk, reset = io.sin_resetn)
 val popCD = ClockDomain(clock = io.sout_clk, reset = io.sout_resetn)
 val fifo = new StreamFifoCC(DType, size, pushCD, popCD)
 fifo.io.push << io.sin
 io.sout << fifo.io.pop
 noIoPrefix()
}
case class MultiClockDemo_P(
 dataW:Int = 32,
 rowN: Int = 4,
 colN: Int = 4,
){
}
case class MultiClockDemo_BaseB(dataW:Int) extends Bundle with IMasterSlave {
 val de = Stream Fragment(UInt(dataW bits))
 val dn = Stream Fragment(UInt(dataW bits))
 val dw = Stream Fragment(UInt(dataW bits))
 val ds = Stream Fragment(UInt(dataW bits))
 override def asMaster(): Unit = {
 master(de )
 master(dn )
 master(dw )
 master(ds )
 }
}
case class MultiClockDemo_B(p:MultiClockDemo_P) extends Bundle with IMasterSlave{
 import p._
 val ds = MultiClockDemo_BaseB(dataW = dataW)
 val dm = MultiClockDemo_BaseB(dataW = dataW)
 override def asMaster(): Unit = {
 master(ds)
 slave(dm)
 }
}
object MultiClockDemo{
 def topoLink(nodes: IndexedSeq[IndexedSeq[MultiClockDemo_B]], p:MultiClockDemo_P) = new AreaRoot{
 import p._
 for (i <- 0 until rowN) {
 for (j <- 0 until colN) {
 if (i > 0) {
 nodes(i)(j).ds.dn << nodes(i - 1)(j).dm.ds
 nodes(i - 1)(j).ds.ds << nodes(i)(j).dm.dn
 }
 if (j > 0) {
 nodes(i)(j).ds.dw << nodes(i)(j - 1).dm.de
 nodes(i)(j - 1).ds.de << nodes(i)(j).dm.dw
 }
 if (i == 0) {
 nodes(0)(j).ds.dn.valid := False
 nodes(0)(j).ds.dn.fragment := 0
 nodes(0)(j).ds.dn.last := False
 nodes(0)(j).dm.dn.ready := True
 }
 if( i == rowN-1){
 nodes(rowN - 1)(j).ds.ds.valid := False
 nodes(rowN - 1)(j).ds.ds.fragment := 0
 nodes(rowN - 1)(j).ds.ds.last := False
 nodes(rowN - 1)(j).dm.ds.ready := True
 }
 if (j == 0) {
 nodes(i)(0).ds.dw.valid := False
 nodes(i)(0).ds.dw.fragment := 0
 nodes(i)(0).ds.dw.last := False
 nodes(i)(0).dm.dw.ready := True
 }
 if (j == colN-1) {
 nodes(i)(colN - 1).ds.de.valid := False
 nodes(i)(colN - 1).ds.de.fragment := 0
 nodes(i)(colN - 1).ds.de.last := False
 nodes(i)(colN - 1).dm.de.ready := True
 }
 }
 }
 }
}
class MultiClockNode(p:MultiClockDemo_P, vcd: Seq[ClockDomain] = null.asInstanceOf[Seq[ClockDomain]]) extends Module {
 import p._
 val drt = slave(MultiClockDemo_B(p))
 val dl_s = slave Stream Fragment(UInt(dataW bits))
 val dl_m = master Stream Fragment(UInt(dataW bits))
 vcd.zipWithIndex.map{case(cd, id) =>
 cd.clock.setName(s"clk${id match {
 case 0 => "E"
 case 1 => "N"
 case 2 => "W"
 case 3 => "S"
 }}_clock")
 cd.reset.setName(s"clk${id match {
 case 0 => "E"
 case 1 => "N"
 case 2 => "W"
 case 3 => "S"
 }}_reset" + (if (cd.config.resetActiveLevel == HIGH) "" else "n"))
 }
 val cd_E = vcd(0)
 val cd_N = vcd(1)
 val cd_W = vcd(2)
 val cd_S = vcd(3)
 val deQ = StreamFifo(drt.ds.de.payloadType, 1).io
 val dnQ = StreamFifo(drt.ds.dn.payloadType, 1).io
 val dwQ = StreamFifo(drt.ds.dw.payloadType, 1).io
 val dsQ = StreamFifo(drt.ds.ds.payloadType, 1).io
 SimpleStreamFifoCC(drt.ds.de, cd_E, popClock = ClockDomain.current) >> deQ.push
 SimpleStreamFifoCC(drt.ds.dn, cd_N, popClock = ClockDomain.current) >> dnQ.push
 SimpleStreamFifoCC(drt.ds.dw, cd_W, popClock = ClockDomain.current) >> dwQ.push
 SimpleStreamFifoCC(drt.ds.ds, cd_S, popClock = ClockDomain.current) >> dsQ.push
 dl_m << StreamArbiterFactory().roundRobin.fragmentLock.buildOn(deQ.pop, dnQ.pop, dwQ.pop, dsQ.pop).io.output
 val outS = StreamFork(dl_s, 4)
 outS(0) >> drt.dm.de
 outS(1) >> drt.dm.dn
 outS(2) >> drt.dm.dw
 outS(3) >> drt.dm.ds
}
class MultiClockDemo(p:MultiClockDemo_P) extends Module {
 import p._
 val cds = IndexedSeq.tabulate(rowN) { r =>
 IndexedSeq.tabulate(colN) { c =>
 val cd_node = ClockDomain.external(s"clk_${r}_${c}")
 cd_node
 }
 }
 val nodes = IndexedSeq.tabulate(rowN) { r =>
 IndexedSeq.tabulate(colN) { c =>
 cds(r)(c) on new AreaRoot {
 val node = new MultiClockNode(p,
 Seq(
 (c < colN - 1).mux(cds(r)(c + 1), cds(r)(c)),
 (r > 0).mux(cds(r - 1)(c), cds(r)(c)),
 (c > 0).mux(cds(r)(c - 1), cds(r)(c)),
 (r < rowN - 1).mux(cds(r + 1)(c), cds(r)(c))
 )
 )
 }.node
 }
 }
 val dnocConn = MultiClockDemo.topoLink(nodes.map(_.map(_.drt)), p)
 val dls_s = for(r <- 0 until rowN;
 c <- 0 until colN) yield nodes(r)(c).dl_s.toIo().setName(s"dl_${r}_${c}_s")
 val dlm_s = for(r <- 0 until rowN;
 c <- 0 until colN) yield nodes(r)(c).dl_m.toIo().setName(s"dl_${r}_${c}_m")
 noIoPrefix()
}

object PhasePullClockDomains{
 def single(c : Component): Unit ={
 val cds = mutable.LinkedHashSet[ClockDomain]()
 c.dslBody.walkLeafStatements{
 case bt : BaseType if bt.isReg =>
 val cd = bt.clockDomain
 if(bt.isUsingResetSignal && (!cd.hasResetSignal && !cd.hasSoftResetSignal))
 SpinalError(s"MISSING RESET SIGNAL in the ClockDomain used by $bt\n${bt.getScalaLocationLong}")

 cds += cd
 case ls => ls.foreachClockDomain(cd => cds += cd)
 }

 c.rework{
 for((cd, id) <- cds.zipWithIndex){
 cd.readClockWire.setName(s"clk${id}_clock")
 if(cd.hasResetSignal) cd.readResetWire.setName(s"clk${id}" + (if(cd.config.resetActiveLevel == LOW) "_resetn" else "reset"))
 if(cd.hasSoftResetSignal) cd.readSoftResetWire
 if(cd.hasClockEnableSignal) cd.readClockEnableWire
 }
 }
 }

 def recursive(c : Component) : Unit = {
 single(c)
 c.children.foreach(recursive)
 }
}

[Dolu1990]

Hi,

Yes right.

It is kinda tricky, because "clk${id}_clock" is kinda a helpless name to give to the pulled clock signal. it will make things lose information.

Maybe an alternative would be to have the possibility to propose a name to something in a component ahead of time, in case it get pulled through ?

[dreamflyings]

Hi,

Yes right.

It is kinda tricky, because "clk${id}_clock" is kinda a helpless name to give to the pulled clock signal. it will make things lose information.

Maybe an alternative would be to have the possibility to propose a name to something in a component ahead of time, in case it get pulled through ?

Good morning or evening, and thank you for your response!

I agree!

The pull function is indeed useful in complex designs or progressive development. However, it has its drawbacks. One major issue is that the naming convention for pull is not intelligent enough—especially in lower-level cells where a more fundamental and unified identifier is essential. This naming inconsistency causes challenges in code reuse.

Additionally, it seems that using pull() might interfere with the effectiveness of the stub() function (although I haven’t personally replicated this issue). Another limitation is that pull cannot be used in reverse or to pull Stream types.

Ideally, having a meaningful and consistent identifier at a certain level of cells or below would be the best solution. However, implementing this may not be straightforward.

[Dolu1990]

Else, one solution would be to improve the way SpinalHDL reconise component duplication.
Instead of comparing the generated verilog to check for duplication, it would need to generate fingerprint using the AST of each component which is independent of how things are named, and use that to check for duplications.

[dreamflyings]

Else, one solution would be to improve the way SpinalHDL reconise component duplication. Instead of comparing the generated verilog to check for duplication, it would need to generate fingerprint using the AST of each component which is independent of how things are named, and use that to check for duplications.

Thank you!
From a first-principles perspective, this is a permanent solution, and this functionality may have other applications as well. However, how challenging would it be to implement this solution, and does it seem somewhat radical? (For example, how can we ensure that different components’ ASTs generate identical fingerprints, and what rules should govern the standardization of their interface names?)

[Dolu1990]

I would no say it is a radical change, as it would interfer in a very specific place.

how can we ensure that different components’ ASTs generate identical fingerprints

Every statement / expression would need to implement a new function which would return a 64 bits hash, then we hash the whole component and compare, if there is a match with already generated component, then we can check statment / expression exactly.

and what rules should govern the standardization of their interface names

I would say, the first component which generate would specify the interface by being its own, the others would remap to it.