[FIRRTL] Handle/Disallow Layer-under-layer that creates bind-under-bind
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
The current lowering of the following produces illegal SystemVerilog that does bind-under-bind:
```
FIRRTL version 4.0.0
circuit Foo: %[[
{"class":"firrtl.transforms.DontTouchAnnotation", "target":"~Foo|Bar>b"},
{"class":"firrtl.transforms.DontTouchAnnotation", "target":"~Foo|Foo>a"}
]]
layer A, bind:
layer B, bind:
module Bar:
layerblock B:
wire b: UInt<1>
invalidate b
module Foo:
layerblock A:
wire a: UInt<1>
invalidate a
inst bar of Bar
```
This produces:
```verilog
// Generated by CIRCT firtool-1.64.0-25-g3dbdd0bc9
module Bar_B();
wire b = 1'h0;
endmodule
module Bar();
endmodule
module Foo_A();
wire a = 1'h0;
Bar bar ();
endmodule
module Foo();
endmodule
// ----- 8< ----- FILE "groups_Foo_B.sv" ----- 8< -----
// Generated by CIRCT firtool-1.64.0-25-g3dbdd0bc9
`ifndef groups_Foo_B
`define groups_Foo_B
bind Bar Bar_B bar_B ();
`endif // groups_Foo_B
// ----- 8< ----- FILE "groups_Foo_A.sv" ----- 8< -----
// Generated by CIRCT firtool-1.64.0-25-g3dbdd0bc9
`ifndef groups_Foo_A
`define groups_Foo_A
bind Foo Foo_A foo_A ();
`endif // groups_Foo_A
```
There are two issues:
1. This should be caught by a verifier after the bind-under-bind is created: https://github.com/llvm/circt/issues/5530
2. The FIRRTL pipeline should disallow this with a verifier in possibly one of two ways:
a. Layerblocks which instantiate modules which transitively have layers should be made illegal.
b. `LowerLayers` needs to work harder to split modules created from layerblocks to avoid this situation entirely.
For 2.b this is hinting that module `Bar` needs to be factored into two units: One which contains only layer A code and one which contains both A and B. Only the one with A code should be bound into `Foo`.
h/t @youngar for identifying this problem
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 with the FIRRTL LowerLayers pipeline and the verifier related to bind-under-bind, using the Foo/Bar reproducer in the issue to reproduce the illegal SystemVerilog. Compare the two proposed directions: rejecting layerblocks that transitively instantiate layers, or factoring the lowered modules so only the valid layer code is bound. Done means the reproducer no longer permits illegal bind-under-bind output.
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
- Mostly clear
- Newbie friendliness
- 25/100