llvm / llvm/circt

[FIRRTL] Handle/Disallow Layer-under-layer that creates bind-under-bind

Open
#6,638 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

FIRRTL
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

  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 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.