[FIRRTL] FIRRTLExporter name collides when operations having same name attr
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
See this mlirblock, it has a valid ssa:
```mlir
#loc = loc(unknown)
"firrtl.circuit"() <{annotations = [], name = "NoName"}> ({
"firrtl.module"() <{annotations = [], layers = [], portAnnotations = [], portDirections = array, portLocations = [#loc, #loc], portNames = ["i", "o"], portSymbols = [], portTypes = [!firrtl.uint<32>, !firrtl.uint<32>], sym_name = "NoName"}> ({
^bb0(%arg0: !firrtl.uint<32>, %arg1: !firrtl.uint<32>):
%0 = "firrtl.wire"() <{name = "io", nameKind = #firrtl}> : () -> !firrtl.bundle, o: uint<32>>
%1 = "firrtl.subfield"(%0) <{fieldIndex = 0 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
"firrtl.connect"(%1, %arg0) : (!firrtl.uint<32>, !firrtl.uint<32>) -> ()
%2 = "firrtl.subfield"(%0) <{fieldIndex = 1 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
"firrtl.connect"(%arg1, %2) : (!firrtl.uint<32>, !firrtl.uint<32>) -> ()
%3 = "firrtl.subfield"(%0) <{fieldIndex = 1 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
%4 = "firrtl.subfield"(%0) <{fieldIndex = 0 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
%5 = "firrtl.subfield"(%0) <{fieldIndex = 0 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
%6 = "firrtl.add"(%4, %5) : (!firrtl.uint<32>, !firrtl.uint<32>) -> !firrtl.uint<33>
%7 = "firrtl.node"(%6) <{name = "tests", nameKind = #firrtl}> : (!firrtl.uint<33>) -> !firrtl.uint<33>
"firrtl.connect"(%3, %7) : (!firrtl.uint<32>, !firrtl.uint<33>) -> ()
%8 = "firrtl.subfield"(%0) <{fieldIndex = 1 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
%9 = "firrtl.subfield"(%0) <{fieldIndex = 0 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
%10 = "firrtl.subfield"(%0) <{fieldIndex = 0 : ui32}> : (!firrtl.bundle, o: uint<32>>) -> !firrtl.uint<32>
%11 = "firrtl.add"(%9, %10) : (!firrtl.uint<32>, !firrtl.uint<32>) -> !firrtl.uint<33>
%12 = "firrtl.node"(%11) <{name = "tests", nameKind = #firrtl}> : (!firrtl.uint<33>) -> !firrtl.uint<33>
"firrtl.connect"(%8, %12) : (!firrtl.uint<32>, !firrtl.uint<33>) -> ()
}) : () -> ()
}) : () -> ()
```
When export firrtl:
```firrtl
circuit NoName :
public module NoName :
input i : UInt<32>
output o : UInt<32>
wire io : { flip i : UInt<32>, o : UInt<32> }
connect io.i, i
connect o, io.o
node tests = add(io.i, io.i) @[/home/sequencer/projects/zaozi/zaozinew/tests/src/Smoke.scala 16]
connect io.o, tests @[/home/sequencer/projects/zaozi/zaozinew/tests/src/Smoke.scala 16]
node tests = add(io.i, io.i) @[/home/sequencer/projects/zaozi/zaozinew/tests/src/Smoke.scala 16]
connect io.o, tests @[/home/sequencer/projects/zaozi/zaozinew/tests/src/Smoke.scala 16]
```
Since the attribute names of `%7` and `%12` are same, node name will collide.
The X-Problem is: in the eDSL designing, managing namespace is not a useful anymore when it can directly refer to the SSA via C-API, I removed the namespace management in [zaozi](https://github.com/sequencer/zaozi) and hit this bug.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the FIRRTLExporter entry point and reproduce the issue with the provided MLIR block, focusing on the two firrtl.node operations that share the name attribute "tests". Trace how operation names are assigned during FIRRTL export and add or run a regression case showing that both nodes receive distinct valid names without breaking their connections.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100