From 85cbc2ec1c370d685013fb7083b52866938fbc3a Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 2 Feb 2023 15:24:43 -0800 Subject: [PATCH 01/24] Implement `typeName` API for a few `Data` types --- core/src/main/scala/chisel3/Data.scala | 3 +++ src/main/scala/chisel3/util/Decoupled.scala | 9 +++++++++ src/main/scala/chisel3/util/Valid.scala | 2 ++ 3 files changed, 14 insertions(+) diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index f89854290f2..0b66a54c253 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -800,6 +800,9 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { /** Default pretty printing */ def toPrintable: Printable + + /** A non-ambiguous name of this `Data` for use in generated Verilog names */ + def typeName: String = this.getClass.getSimpleName } object Data { diff --git a/src/main/scala/chisel3/util/Decoupled.scala b/src/main/scala/chisel3/util/Decoupled.scala index a5cac06e485..fa0cd431e82 100644 --- a/src/main/scala/chisel3/util/Decoupled.scala +++ b/src/main/scala/chisel3/util/Decoupled.scala @@ -40,6 +40,11 @@ abstract class ReadyValidIO[+T <: Data](gen: T) extends Bundle { * @group Signals */ val bits = Output(genType) + + /** A stable typeName for this `ReadyValidIO` and any of its implementations + * using the supplied `Data` generator's `typeName` + */ + override def typeName = s"${this.getClass.simpleName}_${gen.typeName}" } object ReadyValidIO { @@ -334,6 +339,10 @@ class Queue[T <: Data]( Mux(deq_ptr.value > enq_ptr.value, entries.asUInt + ptr_diff, ptr_diff) ) } + + /** Give this Queue a default, stable desired name using the supplied `Data` + * generator's `typeName` */ + override def desiredName = s"Queue${entries}_${gen.typeName}" } /** Factory for a generic hardware queue. */ diff --git a/src/main/scala/chisel3/util/Valid.scala b/src/main/scala/chisel3/util/Valid.scala index 28f56124e01..dc8b620bb20 100644 --- a/src/main/scala/chisel3/util/Valid.scala +++ b/src/main/scala/chisel3/util/Valid.scala @@ -42,6 +42,8 @@ class Valid[+T <: Data](gen: T) extends Bundle { "Chisel 3.5" ) def fire(dummy: Int = 0): Bool = valid + + override def typeName = s"${this.getClass.getSimpleName}_${gen.typeName}" } /** Factory for generating "valid" interfaces. A "valid" interface is a data-communicating interface between a producer From b935729094e921259ea19ace0cd614ac69960031 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 23 Feb 2023 14:48:40 -0800 Subject: [PATCH 02/24] Fix silly compilation error --- src/main/scala/chisel3/util/Decoupled.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/chisel3/util/Decoupled.scala b/src/main/scala/chisel3/util/Decoupled.scala index fa0cd431e82..99b74204788 100644 --- a/src/main/scala/chisel3/util/Decoupled.scala +++ b/src/main/scala/chisel3/util/Decoupled.scala @@ -44,7 +44,7 @@ abstract class ReadyValidIO[+T <: Data](gen: T) extends Bundle { /** A stable typeName for this `ReadyValidIO` and any of its implementations * using the supplied `Data` generator's `typeName` */ - override def typeName = s"${this.getClass.simpleName}_${gen.typeName}" + override def typeName = s"${this.getClass.getSimpleName}_${gen.typeName}" } object ReadyValidIO { From 6a2bf4f90fb5b89703743914ded870123c910132 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 22 Mar 2023 14:41:51 -0700 Subject: [PATCH 03/24] Implement typeName API further for most data types and modules which need it --- core/src/main/scala/chisel3/Aggregate.scala | 4 ++++ core/src/main/scala/chisel3/Bits.scala | 10 ++++++++++ core/src/main/scala/chisel3/Clock.scala | 5 +++++ src/main/scala/chisel3/util/Arbiter.scala | 5 +++++ src/main/scala/chisel3/util/Valid.scala | 12 ++++++++++++ 5 files changed, 36 insertions(+) diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index a6d030613b0..43651fce3fb 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -177,6 +177,10 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend } } + /** Give this Vec a default, stable desired name using the supplied `Data` + * generator's `typeName` */ + override def desiredName = s"Vec${length}_${gen.typeName}" + private[chisel3] override def typeEquivalent(that: Data): Boolean = that match { case that: Vec[T] => this.length == that.length && diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index e735dfe43af..92d8416690d 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -52,6 +52,11 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi def cloneType: this.type = cloneTypeWidth(width) + /** A non-ambiguous name of this `Bits` instance for use in generated Verilog names + * Inserts the width directly after the typeName, e.g. UInt4, SInt1 + */ + def typeName: String = s"${this.getClass.getSimpleName}$width" + /** Tail operator * * @param n the number of bits to remove @@ -1160,6 +1165,11 @@ object AsyncReset { sealed class AsyncReset(private[chisel3] val width: Width = Width(1)) extends Element with Reset { override def toString: String = stringAccessor("AsyncReset") + /** A non-ambiguous name of this `AsyncReset` for use in generated Verilog names + * Inserts the width directly after the typeName, e.g. AsyncReset1 + */ + override def typeName = s"${this.getClass.getSimpleName}$width" + def cloneType: this.type = AsyncReset().asInstanceOf[this.type] private[chisel3] def typeEquivalent(that: Data): Boolean = diff --git a/core/src/main/scala/chisel3/Clock.scala b/core/src/main/scala/chisel3/Clock.scala index 5318c2e5f3e..5f689ebc176 100644 --- a/core/src/main/scala/chisel3/Clock.scala +++ b/core/src/main/scala/chisel3/Clock.scala @@ -17,6 +17,11 @@ object Clock { sealed class Clock(private[chisel3] val width: Width = Width(1)) extends Element { override def toString: String = stringAccessor("Clock") + /** A non-ambiguous name of this `Clock` for use in generated Verilog names + * Inserts the width directly after the typeName, e.g. Clock1 + */ + override def typeName = s"Clock$width" + def cloneType: this.type = Clock().asInstanceOf[this.type] private[chisel3] def typeEquivalent(that: Data): Boolean = diff --git a/src/main/scala/chisel3/util/Arbiter.scala b/src/main/scala/chisel3/util/Arbiter.scala index 9a512157d0f..892a4d95d89 100644 --- a/src/main/scala/chisel3/util/Arbiter.scala +++ b/src/main/scala/chisel3/util/Arbiter.scala @@ -131,6 +131,11 @@ class RRArbiter[T <: Data](val gen: T, val n: Int) extends LockingRRArbiter[T](g * }}} */ class Arbiter[T <: Data](val gen: T, val n: Int) extends Module { + + /** Give this Arbiter a default, stable desired name using the supplied `Data` + * generator's `typeName` and input count parameter */ + override def desiredName = s"Arbiter$n_${gen.typeName}" + val io = IO(new ArbiterIO(gen, n)) io.chosen := (n - 1).asUInt diff --git a/src/main/scala/chisel3/util/Valid.scala b/src/main/scala/chisel3/util/Valid.scala index dc8b620bb20..fca05ce140c 100644 --- a/src/main/scala/chisel3/util/Valid.scala +++ b/src/main/scala/chisel3/util/Valid.scala @@ -43,6 +43,9 @@ class Valid[+T <: Data](gen: T) extends Bundle { ) def fire(dummy: Int = 0): Bool = valid + /** A non-ambiguous name of this `Valid` instance for use in generated Verilog names + * Inserts the parameterized generator's typeName, e.g. Valid_UInt4 + */ override def typeName = s"${this.getClass.getSimpleName}_${gen.typeName}" } @@ -181,6 +184,15 @@ object Pipe { */ class Pipe[T <: Data](val gen: T, val latency: Int = 1)(implicit compileOptions: CompileOptions) extends Module { + /** A non-ambiguous name of this `Pipe` for use in generated Verilog names. + * Includes the cycle count in the name as well as the parameterized generator's + * `typeName`, e.g. `Pipe_4cycles_UInt4` + */ + override def desiredName = { + val latencyStr = s"${latency}cycle${if (latency > 1) "s" else ""}" + s"${this.getClass.getSimpleName}_${latency_str}_${gen.typeName}" + } + /** Interface for [[Pipe]]s composed of a [[Valid]] input and [[Valid]] output * @define notAQueue * @groupdesc Signals Hardware fields of the Bundle From be8f5cf6dc0829aae8e85f18f6ea58f3f04f6be8 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 22 Mar 2023 14:59:18 -0700 Subject: [PATCH 04/24] Compilation error fixes --- core/src/main/scala/chisel3/Aggregate.scala | 2 +- core/src/main/scala/chisel3/Bits.scala | 2 +- src/main/scala/chisel3/util/Arbiter.scala | 2 +- src/main/scala/chisel3/util/Valid.scala | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 43651fce3fb..69225332ca1 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -179,7 +179,7 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend /** Give this Vec a default, stable desired name using the supplied `Data` * generator's `typeName` */ - override def desiredName = s"Vec${length}_${gen.typeName}" + override def typeName = s"Vec${length}_${gen.typeName}" private[chisel3] override def typeEquivalent(that: Data): Boolean = that match { case that: Vec[T] => diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index 92d8416690d..23077aa371d 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -55,7 +55,7 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi /** A non-ambiguous name of this `Bits` instance for use in generated Verilog names * Inserts the width directly after the typeName, e.g. UInt4, SInt1 */ - def typeName: String = s"${this.getClass.getSimpleName}$width" + override def typeName: String = s"${this.getClass.getSimpleName}$width" /** Tail operator * diff --git a/src/main/scala/chisel3/util/Arbiter.scala b/src/main/scala/chisel3/util/Arbiter.scala index 892a4d95d89..6f8028d7136 100644 --- a/src/main/scala/chisel3/util/Arbiter.scala +++ b/src/main/scala/chisel3/util/Arbiter.scala @@ -134,7 +134,7 @@ class Arbiter[T <: Data](val gen: T, val n: Int) extends Module { /** Give this Arbiter a default, stable desired name using the supplied `Data` * generator's `typeName` and input count parameter */ - override def desiredName = s"Arbiter$n_${gen.typeName}" + override def desiredName = s"Arbiter${n}_${gen.typeName}" val io = IO(new ArbiterIO(gen, n)) diff --git a/src/main/scala/chisel3/util/Valid.scala b/src/main/scala/chisel3/util/Valid.scala index fca05ce140c..4c13cc0e7c7 100644 --- a/src/main/scala/chisel3/util/Valid.scala +++ b/src/main/scala/chisel3/util/Valid.scala @@ -190,7 +190,7 @@ class Pipe[T <: Data](val gen: T, val latency: Int = 1)(implicit compileOptions: */ override def desiredName = { val latencyStr = s"${latency}cycle${if (latency > 1) "s" else ""}" - s"${this.getClass.getSimpleName}_${latency_str}_${gen.typeName}" + s"${this.getClass.getSimpleName}_${latencyStr}_${gen.typeName}" } /** Interface for [[Pipe]]s composed of a [[Valid]] input and [[Valid]] output From aab61179b21d68491056b740a0a76ad63b28cbbd Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 22 Mar 2023 15:44:25 -0700 Subject: [PATCH 05/24] Remove width parameter from Bool --- core/src/main/scala/chisel3/Bits.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index 23077aa371d..50f735b3d4a 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -1213,6 +1213,12 @@ sealed class AsyncReset(private[chisel3] val width: Width = Width(1)) extends El * @define numType $coll */ sealed class Bool() extends UInt(1.W) with Reset { + /** + * Give this `Bool` a stable `typeName` for Verilog name generation. + * Specifying a Bool's width in its type name isn't necessary + */ + override def typeName = "Bool" + override def toString: String = { litToBooleanOption match { case Some(value) => s"Bool($value)" From 870320442bb902c20b4c18fae78361b87763fa42 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 22 Mar 2023 15:44:58 -0700 Subject: [PATCH 06/24] Test for Queue naming using typeNames --- .../chiselTests/naming/TypenameSpec.scala | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 src/test/scala/chiselTests/naming/TypenameSpec.scala diff --git a/src/test/scala/chiselTests/naming/TypenameSpec.scala b/src/test/scala/chiselTests/naming/TypenameSpec.scala new file mode 100644 index 00000000000..5005a53f190 --- /dev/null +++ b/src/test/scala/chiselTests/naming/TypenameSpec.scala @@ -0,0 +1,35 @@ +package chiselTests.naming + +import chisel3._ +import chisel3.stage.ChiselStage +import chisel3.util.{Decoupled, Queue} +import chiselTests.ChiselFlatSpec + +class TypenameSpec extends ChiselFlatSpec { + "Queues" should "have stable, type-parameterized default names" in { + class Test extends Module { + val io = IO(new Bundle { + val foo = Decoupled(UInt(4.W)) + val bar = Decoupled(Bool()) + val fizzbuzz = Decoupled(Vec(3, UInt(8.W))) + }) + + val fooEnq = Wire(Decoupled(UInt(4.W))) + val fooDeq = Queue(fooEnq, 16) // Queue16_UInt4 + fooEnq <> io.foo + + val barEnq = Wire(Decoupled(Bool())) + val barDeq = Queue(barEnq, 5) // Queue5_Bool + barEnq <> io.bar + + val fizzbuzzEnq = Wire(Decoupled(Vec(3, UInt(8.W)))) + val fizzbuzzDeq = Queue(fizzbuzzEnq, 32) // Queue32_Vec3_UInt8 + fizzbuzzEnq <> io.fizzbuzz + } + + val chirrtl = ChiselStage.emitChirrtl(new Test) + chirrtl should include("module Queue16_UInt4") + chirrtl should include("module Queue5_Bool") + chirrtl should include("module Queue32_Vec3_UInt8") + } +} From 6dfb9cf3bd578c4ee52d7bf4b19591d1bb20464a Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 23 Mar 2023 15:26:12 -0700 Subject: [PATCH 07/24] Remove width-sensitive override for AsyncReset as it is irrelevant --- core/src/main/scala/chisel3/Bits.scala | 5 ----- core/src/main/scala/chisel3/Clock.scala | 5 ----- 2 files changed, 10 deletions(-) diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index 50f735b3d4a..fea4f0653cf 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -1165,11 +1165,6 @@ object AsyncReset { sealed class AsyncReset(private[chisel3] val width: Width = Width(1)) extends Element with Reset { override def toString: String = stringAccessor("AsyncReset") - /** A non-ambiguous name of this `AsyncReset` for use in generated Verilog names - * Inserts the width directly after the typeName, e.g. AsyncReset1 - */ - override def typeName = s"${this.getClass.getSimpleName}$width" - def cloneType: this.type = AsyncReset().asInstanceOf[this.type] private[chisel3] def typeEquivalent(that: Data): Boolean = diff --git a/core/src/main/scala/chisel3/Clock.scala b/core/src/main/scala/chisel3/Clock.scala index 5f689ebc176..5318c2e5f3e 100644 --- a/core/src/main/scala/chisel3/Clock.scala +++ b/core/src/main/scala/chisel3/Clock.scala @@ -17,11 +17,6 @@ object Clock { sealed class Clock(private[chisel3] val width: Width = Width(1)) extends Element { override def toString: String = stringAccessor("Clock") - /** A non-ambiguous name of this `Clock` for use in generated Verilog names - * Inserts the width directly after the typeName, e.g. Clock1 - */ - override def typeName = s"Clock$width" - def cloneType: this.type = Clock().asInstanceOf[this.type] private[chisel3] def typeEquivalent(that: Data): Boolean = From cbbb8cff60b4c841819d76706cf579a9ff640687 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 27 Mar 2023 13:28:46 -0700 Subject: [PATCH 08/24] Add typeName for Analogs --- core/src/main/scala/chisel3/experimental/Analog.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/scala/chisel3/experimental/Analog.scala b/core/src/main/scala/chisel3/experimental/Analog.scala index 64a538e4c22..75a841352df 100644 --- a/core/src/main/scala/chisel3/experimental/Analog.scala +++ b/core/src/main/scala/chisel3/experimental/Analog.scala @@ -40,6 +40,10 @@ final class Analog private (private[chisel3] val width: Width) extends Element { override def toString: String = stringAccessor(s"Analog$width") + /** A stable typeName for this `Analog` + */ + override def typeName = s"Analog$width" + private[chisel3] override def typeEquivalent(that: Data): Boolean = that.isInstanceOf[Analog] && this.width == that.width From a048eb806d5940140f8a35f74b7ce2d47a50c3e6 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 27 Mar 2023 13:29:01 -0700 Subject: [PATCH 09/24] Add additional types to be tested --- .../chiselTests/naming/TypenameSpec.scala | 60 ++++++++++++++----- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/test/scala/chiselTests/naming/TypenameSpec.scala b/src/test/scala/chiselTests/naming/TypenameSpec.scala index 5005a53f190..190f57ff219 100644 --- a/src/test/scala/chiselTests/naming/TypenameSpec.scala +++ b/src/test/scala/chiselTests/naming/TypenameSpec.scala @@ -1,35 +1,65 @@ package chiselTests.naming import chisel3._ +import chisel3.experimental.Analog import chisel3.stage.ChiselStage import chisel3.util.{Decoupled, Queue} import chiselTests.ChiselFlatSpec -class TypenameSpec extends ChiselFlatSpec { +class TypenameSpec extends ChiselFlatSpec { "Queues" should "have stable, type-parameterized default names" in { class Test extends Module { + class AnalogTest[T <: Analog](gen: T) extends Module { + val bus = IO(gen) + + override def desiredName = s"AnalogTest_${gen.typeName}" + } + val io = IO(new Bundle { - val foo = Decoupled(UInt(4.W)) - val bar = Decoupled(Bool()) - val fizzbuzz = Decoupled(Vec(3, UInt(8.W))) + val uint = Decoupled(UInt(4.W)) + val bool = Decoupled(Bool()) + val vec = Decoupled(Vec(3, UInt(8.W))) + val asyncReset = Decoupled(AsyncReset()) + val reset = Decoupled(Reset()) + val clock = Decoupled(Clock()) + val analog = Analog(32.W) }) - val fooEnq = Wire(Decoupled(UInt(4.W))) - val fooDeq = Queue(fooEnq, 16) // Queue16_UInt4 - fooEnq <> io.foo - - val barEnq = Wire(Decoupled(Bool())) - val barDeq = Queue(barEnq, 5) // Queue5_Bool - barEnq <> io.bar - - val fizzbuzzEnq = Wire(Decoupled(Vec(3, UInt(8.W)))) - val fizzbuzzDeq = Queue(fizzbuzzEnq, 32) // Queue32_Vec3_UInt8 - fizzbuzzEnq <> io.fizzbuzz + val uintEnq = Wire(Decoupled(UInt(4.W))) + val uintDeq = Queue(uintEnq, 16) // Queue16_UInt4 + uintEnq <> io.uint + + val boolEnq = Wire(Decoupled(Bool())) + val boolDeq = Queue(boolEnq, 5) // Queue5_Bool + boolEnq <> io.bool + + val vecEnq = Wire(Decoupled(Vec(3, UInt(8.W)))) + val vecDeq = Queue(vecEnq, 32) // Queue32_Vec3_UInt8 + vecEnq <> io.vec + + val asyncResetEnq = Wire(Decoupled(AsyncReset())) + val asyncResetDeq = Queue(asyncResetEnq, 17) // Queue17_AsyncReset + asyncResetEnq <> io.asyncReset + + val resetEnq = Wire(Decoupled(Reset())) + val resetDeq = Queue(resetEnq, 3) // Queue3_Reset + resetEnq <> io.reset + + val clockEnq = Wire(Decoupled(Clock())) + val clockDeq = Queue(clockEnq, 20) // Queue20_Clock + clockEnq <> io.clock + + val analogTest = Module(new AnalogTest(Analog(32.W))) + analogTest.bus <> io.analog } val chirrtl = ChiselStage.emitChirrtl(new Test) chirrtl should include("module Queue16_UInt4") chirrtl should include("module Queue5_Bool") chirrtl should include("module Queue32_Vec3_UInt8") + chirrtl should include("module Queue17_AsyncReset") + chirrtl should include("module Queue3_Reset") + chirrtl should include("module Queue20_Clock") + chirrtl should include("module AnalogTest_Analog32") } } From 3d8fc0c17305c520907e27673f2f946dda12ec0f Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 27 Mar 2023 13:29:20 -0700 Subject: [PATCH 10/24] Scalafmt --- core/src/main/scala/chisel3/Aggregate.scala | 3 ++- core/src/main/scala/chisel3/Bits.scala | 1 + src/main/scala/chisel3/util/Arbiter.scala | 3 ++- src/main/scala/chisel3/util/Decoupled.scala | 3 ++- src/main/scala/chisel3/util/Valid.scala | 6 +++--- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 69225332ca1..0834a03a85d 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -178,7 +178,8 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend } /** Give this Vec a default, stable desired name using the supplied `Data` - * generator's `typeName` */ + * generator's `typeName` + */ override def typeName = s"Vec${length}_${gen.typeName}" private[chisel3] override def typeEquivalent(that: Data): Boolean = that match { diff --git a/core/src/main/scala/chisel3/Bits.scala b/core/src/main/scala/chisel3/Bits.scala index fea4f0653cf..bd9ed254721 100644 --- a/core/src/main/scala/chisel3/Bits.scala +++ b/core/src/main/scala/chisel3/Bits.scala @@ -1208,6 +1208,7 @@ sealed class AsyncReset(private[chisel3] val width: Width = Width(1)) extends El * @define numType $coll */ sealed class Bool() extends UInt(1.W) with Reset { + /** * Give this `Bool` a stable `typeName` for Verilog name generation. * Specifying a Bool's width in its type name isn't necessary diff --git a/src/main/scala/chisel3/util/Arbiter.scala b/src/main/scala/chisel3/util/Arbiter.scala index 6f8028d7136..3690a84bc4b 100644 --- a/src/main/scala/chisel3/util/Arbiter.scala +++ b/src/main/scala/chisel3/util/Arbiter.scala @@ -133,7 +133,8 @@ class RRArbiter[T <: Data](val gen: T, val n: Int) extends LockingRRArbiter[T](g class Arbiter[T <: Data](val gen: T, val n: Int) extends Module { /** Give this Arbiter a default, stable desired name using the supplied `Data` - * generator's `typeName` and input count parameter */ + * generator's `typeName` and input count parameter + */ override def desiredName = s"Arbiter${n}_${gen.typeName}" val io = IO(new ArbiterIO(gen, n)) diff --git a/src/main/scala/chisel3/util/Decoupled.scala b/src/main/scala/chisel3/util/Decoupled.scala index 99b74204788..0afd91a4344 100644 --- a/src/main/scala/chisel3/util/Decoupled.scala +++ b/src/main/scala/chisel3/util/Decoupled.scala @@ -341,7 +341,8 @@ class Queue[T <: Data]( } /** Give this Queue a default, stable desired name using the supplied `Data` - * generator's `typeName` */ + * generator's `typeName` + */ override def desiredName = s"Queue${entries}_${gen.typeName}" } diff --git a/src/main/scala/chisel3/util/Valid.scala b/src/main/scala/chisel3/util/Valid.scala index 4c13cc0e7c7..6bc5ecfc13c 100644 --- a/src/main/scala/chisel3/util/Valid.scala +++ b/src/main/scala/chisel3/util/Valid.scala @@ -185,9 +185,9 @@ object Pipe { class Pipe[T <: Data](val gen: T, val latency: Int = 1)(implicit compileOptions: CompileOptions) extends Module { /** A non-ambiguous name of this `Pipe` for use in generated Verilog names. - * Includes the cycle count in the name as well as the parameterized generator's - * `typeName`, e.g. `Pipe_4cycles_UInt4` - */ + * Includes the cycle count in the name as well as the parameterized generator's + * `typeName`, e.g. `Pipe_4cycles_UInt4` + */ override def desiredName = { val latencyStr = s"${latency}cycle${if (latency > 1) "s" else ""}" s"${this.getClass.getSimpleName}_${latencyStr}_${gen.typeName}" From 97f2b5cdd9e9f283e5e8f58b22c29255cf157f9d Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 27 Mar 2023 14:52:59 -0700 Subject: [PATCH 11/24] Use circt ChiselStage --- src/test/scala/chiselTests/naming/TypenameSpec.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/scala/chiselTests/naming/TypenameSpec.scala b/src/test/scala/chiselTests/naming/TypenameSpec.scala index 190f57ff219..c464e8e68fd 100644 --- a/src/test/scala/chiselTests/naming/TypenameSpec.scala +++ b/src/test/scala/chiselTests/naming/TypenameSpec.scala @@ -1,10 +1,10 @@ -package chiselTests.naming +package chiselTests +package naming import chisel3._ import chisel3.experimental.Analog -import chisel3.stage.ChiselStage import chisel3.util.{Decoupled, Queue} -import chiselTests.ChiselFlatSpec +import circt.stage.ChiselStage class TypenameSpec extends ChiselFlatSpec { "Queues" should "have stable, type-parameterized default names" in { @@ -53,7 +53,7 @@ class TypenameSpec extends ChiselFlatSpec { analogTest.bus <> io.analog } - val chirrtl = ChiselStage.emitChirrtl(new Test) + val chirrtl = ChiselStage.emitCHIRRTL(new Test) chirrtl should include("module Queue16_UInt4") chirrtl should include("module Queue5_Bool") chirrtl should include("module Queue32_Vec3_UInt8") From 6dd31f1bf6f3012c90d1e000947589ec88b243e5 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 28 Mar 2023 14:40:40 -0700 Subject: [PATCH 12/24] Update queue naming of existing tests --- .../scala/chiselTests/BetterNamingTests.scala | 6 +++++- src/test/scala/chiselTests/CloneModuleSpec.scala | 16 ++++++++-------- src/test/scala/chiselTests/ToTargetSpec.scala | 2 +- .../experimental/DataViewIntegrationSpec.scala | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/test/scala/chiselTests/BetterNamingTests.scala b/src/test/scala/chiselTests/BetterNamingTests.scala index 3aa5dda500d..30267de7fe5 100644 --- a/src/test/scala/chiselTests/BetterNamingTests.scala +++ b/src/test/scala/chiselTests/BetterNamingTests.scala @@ -23,7 +23,11 @@ class PerNameIndexing(count: Int) extends NamedModuleTester { expectModuleName(Module(new Other(i)), genModName("Other", i)) } val queues = Seq.tabulate(count) { i => - expectModuleName(Module(new Queue(UInt(i.W), 16)), genModName("Queue", i)) + expectModuleName(Module(new Queue(UInt(i.W), 16) { + // For this test we need to override desiredName to give the old name, so that indexing + // is properly tested + override def desiredName = "Queue" + }), genModName("Queue", i)) } } diff --git a/src/test/scala/chiselTests/CloneModuleSpec.scala b/src/test/scala/chiselTests/CloneModuleSpec.scala index 303fa8b2339..13f50134570 100644 --- a/src/test/scala/chiselTests/CloneModuleSpec.scala +++ b/src/test/scala/chiselTests/CloneModuleSpec.scala @@ -117,21 +117,21 @@ class CloneModuleSpec extends ChiselPropSpec { } // ********** Checking the output of CloneModuleAsRecord ********** // Note that we overrode desiredName so that Top is named "Top" - mod.q1.io.enq.toTarget.serialize should be("~Top|Queue>io.enq") - mod.q2_io.deq.toTarget.serialize should be("~Top|Queue>io.deq") - mod.q1.io.enq.toAbsoluteTarget.serialize should be("~Top|Top/q1:Queue>io.enq") - mod.q2_io.deq.toAbsoluteTarget.serialize should be("~Top|Top/q2:Queue>io.deq") + mod.q1.io.enq.toTarget.serialize should be("~Top|Queue4_UInt8>io.enq") + mod.q2_io.deq.toTarget.serialize should be("~Top|Queue4_UInt8>io.deq") + mod.q1.io.enq.toAbsoluteTarget.serialize should be("~Top|Top/q1:Queue4_UInt8>io.enq") + mod.q2_io.deq.toAbsoluteTarget.serialize should be("~Top|Top/q2:Queue4_UInt8>io.deq") // Legacy APIs that nevertheless were tricky to get right - mod.q1.io.enq.toNamed.serialize should be("Top.Queue.io.enq") - mod.q2_io.deq.toNamed.serialize should be("Top.Queue.io.deq") + mod.q1.io.enq.toNamed.serialize should be("Top.Queue4_UInt8.io.enq") + mod.q2_io.deq.toNamed.serialize should be("Top.Queue4_UInt8.io.deq") mod.q1.io.enq.instanceName should be("io.enq") mod.q2_io.deq.instanceName should be("io.deq") mod.q1.io.enq.pathName should be("Top.q1.io.enq") mod.q2_io.deq.pathName should be("Top.q2.io.deq") mod.q1.io.enq.parentPathName should be("Top.q1") mod.q2_io.deq.parentPathName should be("Top.q2") - mod.q1.io.enq.parentModName should be("Queue") - mod.q2_io.deq.parentModName should be("Queue") + mod.q1.io.enq.parentModName should be("Queue4_UInt8") + mod.q2_io.deq.parentModName should be("Queue4_UInt8") // ********** Checking the wire cloned from the output of CloneModuleAsRecord ********** val wire_io = mod.q2_wire("io").asInstanceOf[QueueIO[UInt]] diff --git a/src/test/scala/chiselTests/ToTargetSpec.scala b/src/test/scala/chiselTests/ToTargetSpec.scala index d151fc7d973..586211148af 100644 --- a/src/test/scala/chiselTests/ToTargetSpec.scala +++ b/src/test/scala/chiselTests/ToTargetSpec.scala @@ -39,7 +39,7 @@ class ToTargetSpec extends ChiselFlatSpec with Utils { it should "work with modules" in { val q = m.q.toTarget.toString - assert(q == s"~$mn|Queue") + assert(q == s"~$mn|Queue4_UInt32") } it should "error on non-hardware types and provide information" in { diff --git a/src/test/scala/chiselTests/experimental/DataViewIntegrationSpec.scala b/src/test/scala/chiselTests/experimental/DataViewIntegrationSpec.scala index 29d0c533207..efb56f32b76 100644 --- a/src/test/scala/chiselTests/experimental/DataViewIntegrationSpec.scala +++ b/src/test/scala/chiselTests/experimental/DataViewIntegrationSpec.scala @@ -51,6 +51,6 @@ class DataViewIntegrationSpec extends ChiselFlatSpec { "Users" should "be able to view and annotate Modules" in { val (_, annos) = getFirrtlAndAnnos(new MyModule) val ts = annos.collect { case DontTouchAnnotation(t) => t.serialize } - ts should equal(Seq("~MyModule|Queue>enq_ptr_value")) + ts should equal(Seq("~MyModule|Queue4_UInt8>enq_ptr_value")) } } From c531172959108bf8184b45de9d12ee4ba23a2df7 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Tue, 28 Mar 2023 14:45:39 -0700 Subject: [PATCH 13/24] Scalafmt --- src/test/scala/chiselTests/BetterNamingTests.scala | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/test/scala/chiselTests/BetterNamingTests.scala b/src/test/scala/chiselTests/BetterNamingTests.scala index 30267de7fe5..c4c8061a878 100644 --- a/src/test/scala/chiselTests/BetterNamingTests.scala +++ b/src/test/scala/chiselTests/BetterNamingTests.scala @@ -23,11 +23,14 @@ class PerNameIndexing(count: Int) extends NamedModuleTester { expectModuleName(Module(new Other(i)), genModName("Other", i)) } val queues = Seq.tabulate(count) { i => - expectModuleName(Module(new Queue(UInt(i.W), 16) { - // For this test we need to override desiredName to give the old name, so that indexing - // is properly tested - override def desiredName = "Queue" - }), genModName("Queue", i)) + expectModuleName( + Module(new Queue(UInt(i.W), 16) { + // For this test we need to override desiredName to give the old name, so that indexing + // is properly tested + override def desiredName = "Queue" + }), + genModName("Queue", i) + ) } } From a4d00ba10a1ab9f5e670e6aa71836bd35e766818 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 29 Mar 2023 13:19:32 -0700 Subject: [PATCH 14/24] Change pipe desiredName to be consistent with other modules --- src/main/scala/chisel3/util/Valid.scala | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/scala/chisel3/util/Valid.scala b/src/main/scala/chisel3/util/Valid.scala index 05766d23590..4c9f4a36faa 100644 --- a/src/main/scala/chisel3/util/Valid.scala +++ b/src/main/scala/chisel3/util/Valid.scala @@ -179,13 +179,10 @@ object Pipe { class Pipe[T <: Data](val gen: T, val latency: Int = 1) extends Module { /** A non-ambiguous name of this `Pipe` for use in generated Verilog names. - * Includes the cycle count in the name as well as the parameterized generator's - * `typeName`, e.g. `Pipe_4cycles_UInt4` + * Includes the latency cycle count in the name as well as the parameterized + * generator's `typeName`, e.g. `Pipe4_UInt4` */ - override def desiredName = { - val latencyStr = s"${latency}cycle${if (latency > 1) "s" else ""}" - s"${this.getClass.getSimpleName}_${latencyStr}_${gen.typeName}" - } + override def desiredName = s"${this.getClass.getSimpleName}${latency}_${gen.typeName}" /** Interface for [[Pipe]]s composed of a [[Valid]] input and [[Valid]] output * @define notAQueue From 139e5d9041d7a71da9bde4b61e8fc8dfeec8697b Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Wed, 29 Mar 2023 14:21:14 -0700 Subject: [PATCH 15/24] Update naming cookbook --- docs/src/cookbooks/naming.md | 41 +++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index 4e6d26e3f56..ac6f4275d7b 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -72,12 +72,43 @@ ChiselStage.emitSystemVerilog(new ExampleWhenPrefix) `_GEN` signals are usually generated from the FIRRTL compiler, rather than the Chisel library. We are working on renaming these signals with more context-dependent names, but it is a work in progress. Thanks for caring! -### My module names are super unstable - I change one thing and Queue_1 becomes Queue_42. Help! +### My module names are super unstable - I change one thing and Module_1 becomes Module_42. Help! -This is the infamous `Queue` instability problem. In general, these cases are best solved at the source - the module -itself! If you overwrite `desiredName` to include parameter information (see the -[explanation](../explanations/naming#set-a-module-name) for more info), then this can avoid this problem permanantly. -We've done this with some Chisel utilities with great results! +This is an example of the module instability problem, which results from several modules all sharing the exact same name. To fix this, you must add more specificity to your `Module`'s name to avoid these name collisions. + +This can be done by leveraging the `typeName` API, which provides parameter information from a `Data` object for use in a Module's `desiredName` -- thus reducing name collisions and fixing this problem permanently. For instance, suppose your module looks like the following: + +```scala mdoc +class MyModule[T <: Data](gen: T) extends Module { + val in = IO(Input(gen)) + val out = IO(Output(gen)) + out := in +} +``` + +We can override `desiredName` to include the type name of the `gen` parameter like so: + +```scala mdoc +override def desiredName = s"MyModule_${gen.typeName}" +``` + +Any instances of your `MyModule` will now have Verilog module names containing the type parameter. + +```scala mdoc +val foo = Module(new MyModule(UInt(4.W))) // MyModule_UInt4 +val foo = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 +``` + + +All default Chisel modules, like `Queue` and `Pipe`, already have their `desiredName` overrided in this manner, which solves the infamous `Queue` stability problem. + +### I have already overriden `desiredName` to use a `typeName` but my module names are still conflicting! + +You either must add additional information to the `desiredName`, or if you're using your own user-defined `Bundle`, increase the specificity of its own `typeName`. All `Data` types have a simple default implementation of `typeName` (which is simply their own name), but you can override this yourself, of course! + +In general, the suggested pattern for `typeName`, and subsequently `desiredName`, is to fold single integer-like parameters with the name itself (for example, `Queue4`, `UInt3`, `MyBundle9`) and separate these with underscores (`Queue4_UInt3`, `FooBundle_BarType4`). + +Integers should not occur with an underscore before it at the very end of the name (`MyBundle_1`) because this is the _same_ syntax used for duplicates, and so would cause confusion. Having to disambiguate modules all named `FooModule_MyBundle_4_1`, `FooModule_MyBundle_4_2`, `FooModule_MyBundle_4_3`, and so on would be undesirable, indeed! ### I want to add some hardware or assertions, but each time I do all the signal names get bumped! From 9f597b31b71a571ef86442fc43d038a587d2b976 Mon Sep 17 00:00:00 2001 From: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com> Date: Wed, 29 Mar 2023 14:39:33 -0700 Subject: [PATCH 16/24] Update docs/src/cookbooks/naming.md Co-authored-by: Megan Wachs --- docs/src/cookbooks/naming.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index ac6f4275d7b..c4c311c080a 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -76,7 +76,8 @@ renaming these signals with more context-dependent names, but it is a work in pr This is an example of the module instability problem, which results from several modules all sharing the exact same name. To fix this, you must add more specificity to your `Module`'s name to avoid these name collisions. -This can be done by leveraging the `typeName` API, which provides parameter information from a `Data` object for use in a Module's `desiredName` -- thus reducing name collisions and fixing this problem permanently. For instance, suppose your module looks like the following: +This can be done by leveraging the `desiredName` and `typeName` APIs. +`desiredName` is for indicating the names of `Modules` (e.g. influenced by the parameters passed in), and `typeName` is useful for modules which are type-parameterized by subclasses of `Data`. Overriding `desiredName` can reduce or even eliminate name collisions. For instance, suppose your module looks like the following: ```scala mdoc class MyModule[T <: Data](gen: T) extends Module { From acec8ed3802dda067bfba82b09458c4c7794d6ac Mon Sep 17 00:00:00 2001 From: Jared Barocsi <82000041+jared-barocsi@users.noreply.github.com> Date: Wed, 29 Mar 2023 14:40:06 -0700 Subject: [PATCH 17/24] Update docs/src/cookbooks/naming.md Co-authored-by: Megan Wachs --- docs/src/cookbooks/naming.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index c4c311c080a..28819d3968e 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -101,7 +101,7 @@ val foo = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 ``` -All default Chisel modules, like `Queue` and `Pipe`, already have their `desiredName` overrided in this manner, which solves the infamous `Queue` stability problem. +All Chisel built-in library modules, like `Queue` and `Pipe`, already have their `desiredName` overridden in this manner. Assuming that users of these also override the `typeName` of their Data type parameters in a distinct fashion, this will solve the `Queue` stability problem. ### I have already overriden `desiredName` to use a `typeName` but my module names are still conflicting! From b0172cf9ea7d96749efd9aaa8f9fb7a3bd5945e7 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Thu, 30 Mar 2023 14:40:54 -0700 Subject: [PATCH 18/24] Add SInt to TypenameSpec --- src/test/scala/chiselTests/naming/TypenameSpec.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/scala/chiselTests/naming/TypenameSpec.scala b/src/test/scala/chiselTests/naming/TypenameSpec.scala index c464e8e68fd..18c1ec1e4c3 100644 --- a/src/test/scala/chiselTests/naming/TypenameSpec.scala +++ b/src/test/scala/chiselTests/naming/TypenameSpec.scala @@ -17,6 +17,7 @@ class TypenameSpec extends ChiselFlatSpec { val io = IO(new Bundle { val uint = Decoupled(UInt(4.W)) + val sint = Decoupled(SInt(8.W)) val bool = Decoupled(Bool()) val vec = Decoupled(Vec(3, UInt(8.W))) val asyncReset = Decoupled(AsyncReset()) @@ -29,6 +30,10 @@ class TypenameSpec extends ChiselFlatSpec { val uintDeq = Queue(uintEnq, 16) // Queue16_UInt4 uintEnq <> io.uint + val sintEnq = Wire(Decoupled(SInt(8.W))) + val sintDeq = Queue(sintEnq, 8) // Queue8_SInt8 + sintEnq <> io.sint + val boolEnq = Wire(Decoupled(Bool())) val boolDeq = Queue(boolEnq, 5) // Queue5_Bool boolEnq <> io.bool @@ -55,6 +60,7 @@ class TypenameSpec extends ChiselFlatSpec { val chirrtl = ChiselStage.emitCHIRRTL(new Test) chirrtl should include("module Queue16_UInt4") + chirrtl should include("module Queue8_SInt8") chirrtl should include("module Queue5_Bool") chirrtl should include("module Queue32_Vec3_UInt8") chirrtl should include("module Queue17_AsyncReset") From d96670d36a736e6c2cfa82c8280482b8f8a78385 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 3 Apr 2023 12:01:36 -0700 Subject: [PATCH 19/24] Fix cookbook mdoc errors --- docs/src/cookbooks/naming.md | 70 +++++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 13 deletions(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index 28819d3968e..176541159d2 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -79,37 +79,81 @@ This is an example of the module instability problem, which results from several This can be done by leveraging the `desiredName` and `typeName` APIs. `desiredName` is for indicating the names of `Modules` (e.g. influenced by the parameters passed in), and `typeName` is useful for modules which are type-parameterized by subclasses of `Data`. Overriding `desiredName` can reduce or even eliminate name collisions. For instance, suppose your module looks like the following: -```scala mdoc +```scala mdoc:silent class MyModule[T <: Data](gen: T) extends Module { - val in = IO(Input(gen)) - val out = IO(Output(gen)) - out := in + // ... } ``` -We can override `desiredName` to include the type name of the `gen` parameter like so: +We can override `desiredName` of the module to include the type name of the `gen` parameter like so: -```scala mdoc -override def desiredName = s"MyModule_${gen.typeName}" +```scala +override def desiredName = s"MyModule_${gen.typeName} +} ``` Any instances of your `MyModule` will now have Verilog module names containing the type parameter. -```scala mdoc +```scala val foo = Module(new MyModule(UInt(4.W))) // MyModule_UInt4 -val foo = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 +val bar = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 ``` +### Does the `typeName` apply for built-in module names too, most notably `Queue`? + +All Chisel built-in library modules, like `Queue` and `Pipe`, already have their `desiredName` overridden in this manner as well. For instance, one infamous problem arose from instantiating multiple `Queues`: + +```scala +import chisel3._ +import chisel3.util.{Decoupled, Queue} + +val fooDeq = Queue(fooEnq, 8) // Verilog module would be named 'Queue' +val barDeq = Queue(barEnq, 8) // ... and 'Queue_1' +// multiple other Queues... let's assume there are 40 more +val bazDeq = Queue(bazEnq, 8) // ... and finally 'Queue_42' +``` + +By introducing minor changes to the Chisel code, the `Queue` module hierarchy can drastically and unpredictably change -- the `Queue_2` module might rename itself to `Queue_42`, resulting in a lot of confusion and troubles! + +Previously, the recommended solution was to manually override `desiredName` in a `Queue` for extra specificity, but now, both the `typeName` of a `Queue`'s inner type and the Queue's depth are automatically included in its `desiredName`, with *almost* no changes: -All Chisel built-in library modules, like `Queue` and `Pipe`, already have their `desiredName` overridden in this manner. Assuming that users of these also override the `typeName` of their Data type parameters in a distinct fashion, this will solve the `Queue` stability problem. +```scala +val fooQueue = Queue(fooEnq, 8) // Verilog module would be named 'Queue8_UInt8' +val barQueue = Queue(barEnq, 8) // ... and 'Queue8_SInt8' +val bazQueue = Queue(bazEnq, 8) // ... and 'Queue8_Bool' +``` ### I have already overriden `desiredName` to use a `typeName` but my module names are still conflicting! -You either must add additional information to the `desiredName`, or if you're using your own user-defined `Bundle`, increase the specificity of its own `typeName`. All `Data` types have a simple default implementation of `typeName` (which is simply their own name), but you can override this yourself, of course! +In this case, the default `desiredName` or `typeName` implementations for the types you are using aren't sufficient enough to disambiguate conflicting modules. + +You either must add additional information to the `desiredName` of your module, or if you're using your own user-defined `Bundle`, increase the specificity of its own `typeName`. All `Data` types have a simple default implementation of `typeName` (which is simply their own name), but you can override this yourself, of course! + +In general, the suggested pattern for `typeName`, and subsequently `desiredName`, is to fold single integer-like parameters with the name itself (for example, `Queue4`, `UInt3`, `MyBundle9`) to form 'words' and separate these 'words' with underscores (`Queue4_UInt3`, `FooBundle_BarType4`). The following example shows how to construct such a `typeName` with a simple parameterized bundle: + +```scala mdoc:silent +import chisel3._ + +class MyBundle[T <: Data](gen: T, intParam: Int) extends Bundle { + // Generate a stable typeName for this Bundle. Two 'words' are present + // in this implementation: the bundle's name plus its integer parameter + // (something like 'MyBundle9') + // and the generator's typeName, which itself can be composed of 'words' + // (something like 'Vec3_UInt4') + override def typeName = s"MyBundle${intParam}_${gen.typeName}" + + // ... +} +``` + +```scala mdoc:silent +val example1 = new MyBundle(Bool(), 1) // MyBundle1_Bool +val example2 = new MyBundle(Vec(3, UInt(4.W)), 9) // MyBundle9_Vec3_UInt4 +``` -In general, the suggested pattern for `typeName`, and subsequently `desiredName`, is to fold single integer-like parameters with the name itself (for example, `Queue4`, `UInt3`, `MyBundle9`) and separate these with underscores (`Queue4_UInt3`, `FooBundle_BarType4`). +`Bundles` that have multiple integer arguments aren't presently addressed by any of the built-in modules, and so implementing a descriptive and sufficiently differentiable `typeName` for such `Bundles` is left as an exercise to the reader. -Integers should not occur with an underscore before it at the very end of the name (`MyBundle_1`) because this is the _same_ syntax used for duplicates, and so would cause confusion. Having to disambiguate modules all named `FooModule_MyBundle_4_1`, `FooModule_MyBundle_4_2`, `FooModule_MyBundle_4_3`, and so on would be undesirable, indeed! +Integers should not occur with an underscore before them at the very end of the `typeName` (`MyBundle_1`) because this is the _same_ syntax used for duplicates, and so would cause confusion. Having to disambiguate modules all named `Queue32_MyBundle_4_1`, `Queue32_MyBundle_4_2`, `Queue32_MyBundle_4_3`, and so on would be undesirable, indeed! ### I want to add some hardware or assertions, but each time I do all the signal names get bumped! From 80c1218022e8ab9cdb919965bdf6cbc6c1e3a79e Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 3 Apr 2023 13:54:21 -0700 Subject: [PATCH 20/24] compileOnly tag on scala code --- docs/src/cookbooks/naming.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index 176541159d2..ce8e2360ad5 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -94,7 +94,7 @@ override def desiredName = s"MyModule_${gen.typeName} Any instances of your `MyModule` will now have Verilog module names containing the type parameter. -```scala +```scala mdoc:compileOnly val foo = Module(new MyModule(UInt(4.W))) // MyModule_UInt4 val bar = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 ``` From 3556190c7fc6cb3b8f904318a1657c7ffe7f56d0 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 3 Apr 2023 14:29:38 -0700 Subject: [PATCH 21/24] More improvements of mdoc --- docs/src/cookbooks/naming.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index ce8e2360ad5..14b235df1bc 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -87,8 +87,11 @@ class MyModule[T <: Data](gen: T) extends Module { We can override `desiredName` of the module to include the type name of the `gen` parameter like so: -```scala -override def desiredName = s"MyModule_${gen.typeName} +```scala mdoc:silent:reset +import chisel3._ + +class MyModule[T <: Data](gen: T) extends Module { + override def desiredName = s"MyModule_${gen.typeName}" } ``` From 5e6bca7635574bce1c20bde28643edcc5f1fda91 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 3 Apr 2023 14:45:44 -0700 Subject: [PATCH 22/24] compileOnly -> compile-only --- docs/src/cookbooks/naming.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index 14b235df1bc..e5bf746732b 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -97,7 +97,7 @@ class MyModule[T <: Data](gen: T) extends Module { Any instances of your `MyModule` will now have Verilog module names containing the type parameter. -```scala mdoc:compileOnly +```scala mdoc:compile-only val foo = Module(new MyModule(UInt(4.W))) // MyModule_UInt4 val bar = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 ``` From 091ab72c5c4e087404e50db0d835e730a8222340 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 3 Apr 2023 15:26:48 -0700 Subject: [PATCH 23/24] Final mdoc review and fixes --- docs/src/cookbooks/naming.md | 59 +++++++++++++----------------------- 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/docs/src/cookbooks/naming.md b/docs/src/cookbooks/naming.md index e5bf746732b..37c7b4e4572 100644 --- a/docs/src/cookbooks/naming.md +++ b/docs/src/cookbooks/naming.md @@ -72,7 +72,7 @@ ChiselStage.emitSystemVerilog(new ExampleWhenPrefix) `_GEN` signals are usually generated from the FIRRTL compiler, rather than the Chisel library. We are working on renaming these signals with more context-dependent names, but it is a work in progress. Thanks for caring! -### My module names are super unstable - I change one thing and Module_1 becomes Module_42. Help! +### How do I make my modules have more stable names instead of 'Module_1' and 'Module_42'? This is an example of the module instability problem, which results from several modules all sharing the exact same name. To fix this, you must add more specificity to your `Module`'s name to avoid these name collisions. @@ -87,9 +87,12 @@ class MyModule[T <: Data](gen: T) extends Module { We can override `desiredName` of the module to include the type name of the `gen` parameter like so: -```scala mdoc:silent:reset +```scala mdoc:invisible:reset import chisel3._ +import chisel3.util.Queue +``` +```scala mdoc class MyModule[T <: Data](gen: T) extends Module { override def desiredName = s"MyModule_${gen.typeName}" } @@ -102,41 +105,19 @@ val foo = Module(new MyModule(UInt(4.W))) // MyModule_UInt4 val bar = Module(new MyModule(Vec(3, UInt(4.W)))) // MyModule_Vec3_UInt4 ``` -### Does the `typeName` apply for built-in module names too, most notably `Queue`? - -All Chisel built-in library modules, like `Queue` and `Pipe`, already have their `desiredName` overridden in this manner as well. For instance, one infamous problem arose from instantiating multiple `Queues`: - -```scala -import chisel3._ -import chisel3.util.{Decoupled, Queue} - -val fooDeq = Queue(fooEnq, 8) // Verilog module would be named 'Queue' -val barDeq = Queue(barEnq, 8) // ... and 'Queue_1' -// multiple other Queues... let's assume there are 40 more -val bazDeq = Queue(bazEnq, 8) // ... and finally 'Queue_42' -``` - -By introducing minor changes to the Chisel code, the `Queue` module hierarchy can drastically and unpredictably change -- the `Queue_2` module might rename itself to `Queue_42`, resulting in a lot of confusion and troubles! - -Previously, the recommended solution was to manually override `desiredName` in a `Queue` for extra specificity, but now, both the `typeName` of a `Queue`'s inner type and the Queue's depth are automatically included in its `desiredName`, with *almost* no changes: +Note that all base Chisel util modules, like `Queue`, already implement `desiredName` like this: -```scala -val fooQueue = Queue(fooEnq, 8) // Verilog module would be named 'Queue8_UInt8' -val barQueue = Queue(barEnq, 8) // ... and 'Queue8_SInt8' -val bazQueue = Queue(bazEnq, 8) // ... and 'Queue8_Bool' +```scala mdoc:compile-only +val fooQueue = Module(new Queue(UInt(8.W), 4)) // Verilog module would be named 'Queue4_UInt8' +val barQueue = Module(new Queue(SInt(12.W), 3)) // ... and 'Queue3_SInt12' +val bazQueue = Module(new Queue(Bool(), 16)) // ... and 'Queue16_Bool' ``` -### I have already overriden `desiredName` to use a `typeName` but my module names are still conflicting! - -In this case, the default `desiredName` or `typeName` implementations for the types you are using aren't sufficient enough to disambiguate conflicting modules. - -You either must add additional information to the `desiredName` of your module, or if you're using your own user-defined `Bundle`, increase the specificity of its own `typeName`. All `Data` types have a simple default implementation of `typeName` (which is simply their own name), but you can override this yourself, of course! +### How would I write my own `typeName` for my data types? -In general, the suggested pattern for `typeName`, and subsequently `desiredName`, is to fold single integer-like parameters with the name itself (for example, `Queue4`, `UInt3`, `MyBundle9`) to form 'words' and separate these 'words' with underscores (`Queue4_UInt3`, `FooBundle_BarType4`). The following example shows how to construct such a `typeName` with a simple parameterized bundle: +If you're using your own user-defined `Bundle`, you can increase the specificity of its own `typeName` by overriding it. All `Data` types have a simple default implementation of `typeName` (which is simply its class name), but you can override this yourself: ```scala mdoc:silent -import chisel3._ - class MyBundle[T <: Data](gen: T, intParam: Int) extends Bundle { // Generate a stable typeName for this Bundle. Two 'words' are present // in this implementation: the bundle's name plus its integer parameter @@ -149,14 +130,15 @@ class MyBundle[T <: Data](gen: T, intParam: Int) extends Bundle { } ``` -```scala mdoc:silent -val example1 = new MyBundle(Bool(), 1) // MyBundle1_Bool -val example2 = new MyBundle(Vec(3, UInt(4.W)), 9) // MyBundle9_Vec3_UInt4 +Now if you use your `MyBundle` in a module like a `Queue`: + +```scala mdoc:compile-only +val fooQueue = Module(new Queue(new MyBundle(UInt(4.W), 3), 16)) // Queue16_MyBundle3_UInt4 ``` -`Bundles` that have multiple integer arguments aren't presently addressed by any of the built-in modules, and so implementing a descriptive and sufficiently differentiable `typeName` for such `Bundles` is left as an exercise to the reader. +The suggested pattern for `typeName`, and subsequently `desiredName`, is to fold single integer-like parameters with the name itself (for example, `Queue4`, `UInt3`, `MyBundle9`) to form 'words' and separate these 'words' with underscores (`Queue4_UInt3`, `FooBundle_BarType4`). -Integers should not occur with an underscore before them at the very end of the `typeName` (`MyBundle_1`) because this is the _same_ syntax used for duplicates, and so would cause confusion. Having to disambiguate modules all named `Queue32_MyBundle_4_1`, `Queue32_MyBundle_4_2`, `Queue32_MyBundle_4_3`, and so on would be undesirable, indeed! +`Bundles` that have multiple integer arguments aren't presently addressed by any of the built-in modules, and so implementing a descriptive and sufficiently differentiable `typeName` for such `Bundles` is left as an exercise to the reader. However, integers should not occur with an underscore before them at the very end of the `typeName` (e.g. `MyBundle_1`) because this is the _same_ syntax used for duplicates, and so would cause confusion. Having to disambiguate modules all named `Queue32_MyBundle_4_1`, `Queue32_MyBundle_4_2`, `Queue32_MyBundle_4_3`, and so on would be undesirable, indeed! ### I want to add some hardware or assertions, but each time I do all the signal names get bumped! @@ -184,8 +166,9 @@ class ExampleNoPrefix extends Module { out := add } ``` + ```scala mdoc:verilog -ChiselStage.emitSystemVerilog(new ExampleNoPrefix) +emitVerilog(new ExampleNoPrefix) ``` ### I am still not getting the name I want. For example, inlining an instance changes my name! @@ -217,7 +200,7 @@ class MyLeaf extends Module { } ``` ```scala mdoc:verilog -ChiselStage.emitSystemVerilog(new WrapperExample) +emitVerilog(new WrapperExample) ``` This can be used to rename instances and non-aggregate typed signals. From a48a6b9b06e6d60f227f2614ae9cb591d9fb8e16 Mon Sep 17 00:00:00 2001 From: Jared Barocsi Date: Mon, 3 Apr 2023 17:55:09 -0700 Subject: [PATCH 24/24] Add inferred UInt to test --- src/test/scala/chiselTests/naming/TypenameSpec.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/scala/chiselTests/naming/TypenameSpec.scala b/src/test/scala/chiselTests/naming/TypenameSpec.scala index 18c1ec1e4c3..b429dfe0296 100644 --- a/src/test/scala/chiselTests/naming/TypenameSpec.scala +++ b/src/test/scala/chiselTests/naming/TypenameSpec.scala @@ -30,6 +30,10 @@ class TypenameSpec extends ChiselFlatSpec { val uintDeq = Queue(uintEnq, 16) // Queue16_UInt4 uintEnq <> io.uint + val uintInferredEnq = Wire(Decoupled(UInt())) + val uintInferredDeq = Queue(uintInferredEnq, 15) // Queue16_UInt + uintInferredEnq <> io.uint + val sintEnq = Wire(Decoupled(SInt(8.W))) val sintDeq = Queue(sintEnq, 8) // Queue8_SInt8 sintEnq <> io.sint @@ -60,6 +64,7 @@ class TypenameSpec extends ChiselFlatSpec { val chirrtl = ChiselStage.emitCHIRRTL(new Test) chirrtl should include("module Queue16_UInt4") + chirrtl should include("module Queue15_UInt") chirrtl should include("module Queue8_SInt8") chirrtl should include("module Queue5_Bool") chirrtl should include("module Queue32_Vec3_UInt8")