Skip to content

Commit 3ef3ea2

Browse files
Fix naming for RHS of named unapply expressions (backport #3163) (#3165)
* Fix naming for RHS of named unapply expressions (#3163) Previously, the naming plugin would not recurse for the RHS of an unapply, meaning that vals declared in blocks that ultimately return some result that is unapplied into some named Data would not be named. Also, audit and improve the naming mdocs. (cherry picked from commit 262caa7) # Conflicts: # docs/src/cookbooks/naming.md * Resolve backport conflicts --------- Co-authored-by: Jack Koenig <koenig@sifive.com>
1 parent 05be8a2 commit 3ef3ea2

File tree

4 files changed

+71
-34
lines changed

4 files changed

+71
-34
lines changed

docs/src/cookbooks/naming.md

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ section: "chisel3"
77
```scala mdoc:invisible
88
import chisel3._
99
import circt.stage.ChiselStage
10+
def emitSystemVerilog(gen: => RawModule): String = {
11+
val prettyArgs = Array("--disable-all-randomization", "--strip-debug-info")
12+
ChiselStage.emitSystemVerilog(gen, firtoolOpts = prettyArgs)
13+
}
1014
```
1115
# Naming Cookbook
1216

@@ -28,8 +32,8 @@ We recommend you manually insert calls to `prefix` to clarify these cases:
2832
import chisel3.experimental.prefix
2933
class ExamplePrefix extends Module {
3034

31-
Seq.tabulate{2} {i =>
32-
Seq.tabulate{2}{ j =>
35+
Seq.tabulate(2) { i =>
36+
Seq.tabulate(2) { j =>
3337
prefix(s"loop_${i}_${j}"){
3438
val x = WireInit((i*0x10+j).U(8.W))
3539
dontTouch(x)
@@ -39,7 +43,7 @@ class ExamplePrefix extends Module {
3943
}
4044
```
4145
```scala mdoc:verilog
42-
ChiselStage.emitSystemVerilog(new ExamplePrefix)
46+
emitSystemVerilog(new ExamplePrefix)
4347
```
4448
### How can I get better names for code generated by `when` clauses?
4549

@@ -53,18 +57,20 @@ class ExampleWhenPrefix extends Module {
5357

5458
out := DontCare
5559

56-
Seq.tabulate{2}{ i =>
60+
Seq.tabulate(2) { i =>
5761
val j = i + 1
58-
when (in === j.U) { prefix(s"clause_${j}"){
59-
val foo = Wire(UInt(4.W))
60-
foo := in +& j.U(4.W)
61-
out := foo
62-
}}
62+
prefix(s"clause_${j}") {
63+
when (in === j.U) {
64+
val foo = Reg(UInt(4.W))
65+
foo := in + j.U(4.W)
66+
out := foo
67+
}
68+
}
6369
}
6470
}
6571
```
6672
```scala mdoc:verilog
67-
ChiselStage.emitSystemVerilog(new ExampleWhenPrefix)
73+
emitSystemVerilog(new ExampleWhenPrefix)
6874
```
6975

7076
### I still see _GEN signals, can this be fixed?
@@ -100,13 +106,17 @@ class ExampleNoPrefix extends Module {
100106
val in = IO(Input(UInt(2.W)))
101107
val out = IO(Output(UInt()))
102108

103-
val add = noPrefix { in + in + in }
109+
val add = noPrefix {
110+
// foo will not get a prefix
111+
val foo = RegNext(in + 1.U)
112+
foo + in
113+
}
104114

105115
out := add
106116
}
107117
```
108118
```scala mdoc:verilog
109-
ChiselStage.emitSystemVerilog(new ExampleNoPrefix)
119+
emitSystemVerilog(new ExampleNoPrefix)
110120
```
111121

112122
### I am still not getting the name I want. For example, inlining an instance changes my name!
@@ -138,7 +148,7 @@ class MyLeaf extends Module {
138148
}
139149
```
140150
```scala mdoc:verilog
141-
ChiselStage.emitSystemVerilog(new WrapperExample)
151+
emitSystemVerilog(new WrapperExample)
142152
```
143153

144154
This can be used to rename instances and non-aggregate typed signals.

docs/src/explanations/naming.md

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,14 @@ systemic name-stability issues, please refer to the naming [cookbook](../cookboo
2121
// Imports used by the following examples
2222
import chisel3._
2323
import chisel3.experimental.{prefix, noPrefix}
24+
```
25+
26+
```scala mdoc:invisible
2427
import circt.stage.ChiselStage
28+
def emitSystemVerilog(gen: => RawModule): String = {
29+
val prettyArgs = Array("--disable-all-randomization", "--strip-debug-info")
30+
ChiselStage.emitSystemVerilog(gen, firtoolOpts = prettyArgs)
31+
}
2532
```
2633

2734
With the release of Chisel 3.5, users are required to add the following line to
@@ -51,7 +58,7 @@ class Example1 extends Module {
5158
}
5259
```
5360
```scala mdoc:verilog
54-
ChiselStage.emitSystemVerilog(new Example1)
61+
emitSystemVerilog(new Example1)
5562
```
5663

5764
Otherwise, it is rewritten to also include the name as a prefix to any signals generated while executing the right-hand-
@@ -75,7 +82,7 @@ class Example2 extends Module {
7582
}
7683
```
7784
```scala mdoc:verilog
78-
ChiselStage.emitSystemVerilog(new Example2)
85+
emitSystemVerilog(new Example2)
7986
```
8087

8188
Prefixing can also be derived from the name of signals on the left-hand side of a connection.
@@ -98,7 +105,7 @@ class ConnectPrefixing extends Module {
98105
}
99106
```
100107
```scala mdoc:verilog
101-
ChiselStage.emitSystemVerilog(new ConnectPrefixing)
108+
emitSystemVerilog(new ConnectPrefixing)
102109
```
103110

104111
Note that the naming also works if the hardware type is nested in an `Option` or a subtype of `Iterable`:
@@ -109,19 +116,22 @@ class Example3 extends Module {
109116
// val in = autoNameRecursively("in")(prefix("in")(IO(Input(UInt(2.W)))))
110117

111118
val out = IO(Output(UInt()))
112-
// val out = autoNameRecursively("out")(prefix("out")(IO(Output(UInt(2.W)))))
119+
// val out = autoNameRecursively("out")(prefix("out")(IO(Output(UInt()))))
113120

114-
def inXin() = in * in
121+
def func() = {
122+
val delay = RegNext(in)
123+
delay + 1.U
124+
}
115125

116-
val opt = Some(3.U + inXin())
117-
// Note that the intermediate result of the inXin() is prefixed with `opt`:
118-
// val opt = autoNameRecursively("opt")(prefix("opt")(Some(3.U + inXin())))
126+
val opt = Some(func())
127+
// Note that the register in func() is prefixed with `opt`:
128+
// val opt = autoNameRecursively("opt")(prefix("opt")(Some(func()))
119129

120130
out := opt.get + 1.U
121131
}
122132
```
123133
```scala mdoc:verilog
124-
ChiselStage.emitSystemVerilog(new Example3)
134+
emitSystemVerilog(new Example3)
125135
```
126136

127137
There is also a slight variant (`autoNameRecursivelyProduct`) for naming hardware with names provided by an unapply:
@@ -135,7 +145,7 @@ class UnapplyExample extends Module {
135145
}
136146
```
137147
```scala mdoc:verilog
138-
ChiselStage.emitSystemVerilog(new UnapplyExample)
148+
emitSystemVerilog(new UnapplyExample)
139149
```
140150

141151
Note that the compiler plugin will not insert a prefix in these cases because it is ambiguous what the prefix should be.
@@ -169,8 +179,8 @@ class Example5 extends Module {
169179
}
170180
```
171181
```scala mdoc:verilog
172-
ChiselStage.emitSystemVerilog(new Example4)
173-
ChiselStage.emitSystemVerilog(new Example5)
182+
emitSystemVerilog(new Example4)
183+
emitSystemVerilog(new Example5)
174184
```
175185

176186
Also note that the prefixes append to each other (including the prefix generated by the compiler plugin):
@@ -186,7 +196,7 @@ class Example6 extends Module {
186196
}
187197
```
188198
```scala mdoc:verilog
189-
ChiselStage.emitSystemVerilog(new Example6)
199+
emitSystemVerilog(new Example6)
190200
```
191201

192202
Sometimes you may want to disable the prefixing. This might occur if you are writing a library function and
@@ -203,7 +213,7 @@ class Example7 extends Module {
203213
}
204214
```
205215
```scala mdoc:verilog
206-
ChiselStage.emitSystemVerilog(new Example7)
216+
emitSystemVerilog(new Example7)
207217
```
208218

209219
### Suggest a Signal's Name (or the instance name of a Module)
@@ -222,7 +232,7 @@ class Example8 extends Module {
222232
}
223233
```
224234
```scala mdoc:verilog
225-
ChiselStage.emitSystemVerilog(new Example8)
235+
emitSystemVerilog(new Example8)
226236
```
227237

228238
Note that using `.suggestName` does **not** affect prefixes derived from val names;
@@ -265,7 +275,7 @@ class ConnectionPrefixExample extends Module {
265275
}
266276
```
267277
```scala mdoc:verilog
268-
ChiselStage.emitSystemVerilog(new ConnectionPrefixExample)
278+
emitSystemVerilog(new ConnectionPrefixExample)
269279
```
270280

271281
As this example illustrates, this behavior is slightly inconsistent so is subject to change in a future version of Chisel.
@@ -291,7 +301,7 @@ class TemporaryExample extends Module {
291301
}
292302
```
293303
```scala mdoc:verilog
294-
ChiselStage.emitSystemVerilog(new TemporaryExample)
304+
emitSystemVerilog(new TemporaryExample)
295305
```
296306

297307
If an unnamed signal is itself used to generate a prefix, the leading `_` will be ignored to avoid double `__` in the names of further nested signals.
@@ -311,7 +321,7 @@ class TemporaryPrefixExample extends Module {
311321
}
312322
```
313323
```scala mdoc:verilog
314-
ChiselStage.emitSystemVerilog(new TemporaryPrefixExample)
324+
emitSystemVerilog(new TemporaryPrefixExample)
315325
```
316326

317327

@@ -333,8 +343,8 @@ class Example9(width: Int) extends Module {
333343
}
334344
```
335345
```scala mdoc:verilog
336-
ChiselStage.emitSystemVerilog(new Example9(8))
337-
ChiselStage.emitSystemVerilog(new Example9(1))
346+
emitSystemVerilog(new Example9(8))
347+
emitSystemVerilog(new Example9(1))
338348
```
339349

340350
### Reflection Naming

plugin/src/main/scala/chisel3/internal/plugin/ChiselComponent.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ class ChiselComponent(val global: Global, arguments: ChiselPluginArguments)
233233
case Some(names) =>
234234
val onames: List[Option[String]] =
235235
fieldsOfInterest.zip(names).map { case (ok, name) => if (ok) Some(name) else None }
236-
val named = q"chisel3.internal.plugin.autoNameRecursivelyProduct($onames)($rhs)"
236+
val newRHS = transform(rhs)
237+
val named = q"chisel3.internal.plugin.autoNameRecursivelyProduct($onames)($newRHS)"
237238
treeCopy.ValDef(dd, mods, name, tpt, localTyper.typed(named))
238239
case None => // It's not clear how this could happen but we don't want to crash
239240
super.transform(tree)

src/test/scala/chiselTests/naming/NamePluginSpec.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,22 @@ class NamePluginSpec extends ChiselFlatSpec with Utils {
267267
}
268268
}
269269

270+
"Unapply assignments" should "name (but not prefix) local vals on the RHS" in {
271+
class Test extends Module {
272+
{
273+
val (a, b) = {
274+
val x, y = Wire(UInt(3.W))
275+
val sum = WireInit(x + y)
276+
(x, y)
277+
}
278+
}
279+
}
280+
281+
aspectTest(() => new Test) { top: Test =>
282+
Select.wires(top).map(_.instanceName) should be(List("a", "b", "sum"))
283+
}
284+
}
285+
270286
"Unapply assignments" should "not override already named things" in {
271287
class Test extends Module {
272288
{

0 commit comments

Comments
 (0)