llvm / llvm/circt

[FIRRTL] FIRRTLExporter name collides when operations having same name attr

Open
#8,027 0 comments 0 reactions 0 assignees View on GitHub

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.