[FIRParser] behavior with multiple circuits
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
# Summary
We currently accept multiple top-level circuit definitions when parsing FIRRTL and (IMO) produce unexpected IR when this is encountered. The presence of multiple circuits in one file does not appear to be valid FIRRTL, and as a result I think this should either be rejected, warned on, or supported in a more expected way (regardless of being in the spec). Personally I'd prefer to see it supported, but am unsure about the current behavior.
Presently we appear to ignore subsequent circuit declarations after the first and add any modules within to the first circuit (as if the later "circuit" lines were not present). An example helps demonstrate:
# Example
Example (`test.fir`):
```firrtl
circuit Top1:
module Top1:
skip
circuit Top2:
module Top2:
skip
```
## Current Behavior
Running `firtool --parse-only -format=fir test.fir` produces:
```mlir
module {
firrtl.circuit "Top1" {
firrtl.module @Top1() {
firrtl.skip
}
firrtl.module private @Top2() {
firrtl.skip
}
}
}
```
Notice there is no "Top2" circuit emitted, and its sole module (`@Top2`) is added into the first circuit.
## Possible alternate behavior
If supported, I'd expect something like this be produced:
```mlir
module {
firrtl.circuit "Top1" {
firrtl.module @Top1() {
firrtl.skip
}
}
firrtl.circuit "Top2" {
firrtl.module @Top2() {
firrtl.skip
}
}
}
```
That is, all circuits in the source are present and modules are within their respective circuits.
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
Reproduce the behavior with the test.fir example using `firtool --parse-only -format=fir test.fir` and compare the emitted MLIR with the issue's examples. Determine whether multiple top-level circuits should be supported, rejected, or warned on; done means the chosen behavior is implemented consistently rather than silently merging Top2 into Top1.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100