llvm / llvm/circt

[FIRRTL] FART: Does not process modules not reachable from the main module

Open
#7,674 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug FIRRTL
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

Two identical modules, both annotated with FART:
```FIRRTL
FIRRTL version 4.0.0
circuit Foo: %[[
{"class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation", "target":"~Foo|Foo>r"},
{"class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation", "target":"~Foo|Bar>r"}
]]
public module Foo:
input c : Clock
input i : UInt<8>
input r : AsyncReset
output o : UInt<8>
reg reg : UInt<8>, c
connect reg, i
connect o, reg

public module Bar:
input c : Clock
input i : UInt<8>
input r : AsyncReset
output o : UInt<8>
reg reg : UInt<8>, c
connect reg, i
connect o, reg
```
gives:
```verilog
module Foo(
input c,
input [7:0] i,
input r,
output [7:0] o
);

reg [7:0] reg_0;
always @(posedge c or posedge r) begin
if (r)
reg_0 <= 8'h0;
else
reg_0 <= i;
end // always @(posedge, posedge)
`ifdef ENABLE_INITIAL_REG_
`ifdef FIRRTL_BEFORE_INITIAL
`FIRRTL_BEFORE_INITIAL
`endif // FIRRTL_BEFORE_INITIAL
initial begin
automatic logic [31:0] _RANDOM[0:0];
`ifdef INIT_RANDOM_PROLOG_
`INIT_RANDOM_PROLOG_
`endif // INIT_RANDOM_PROLOG_
`ifdef RANDOMIZE_REG_INIT
_RANDOM[/*Zero width*/ 1'b0] = `RANDOM;
reg_0 = _RANDOM[/*Zero width*/ 1'b0][7:0];
`endif // RANDOMIZE_REG_INIT
if (r)
reg_0 = 8'h0;
end // initial
`ifdef FIRRTL_AFTER_INITIAL
`FIRRTL_AFTER_INITIAL
`endif // FIRRTL_AFTER_INITIAL
`endif // ENABLE_INITIAL_REG_
assign o = reg_0;
endmodule

module Bar(
input c,
input [7:0] i,
input r,
output [7:0] o
);

reg [7:0] reg_0;
always @(posedge c)
reg_0 <= i;
`ifdef ENABLE_INITIAL_REG_
`ifdef FIRRTL_BEFORE_INITIAL
`FIRRTL_BEFORE_INITIAL
`endif // FIRRTL_BEFORE_INITIAL
initial begin
automatic logic [31:0] _RANDOM[0:0];
`ifdef INIT_RANDOM_PROLOG_
`INIT_RANDOM_PROLOG_
`endif // INIT_RANDOM_PROLOG_
`ifdef RANDOMIZE_REG_INIT
_RANDOM[/*Zero width*/ 1'b0] = `RANDOM;
reg_0 = _RANDOM[/*Zero width*/ 1'b0][7:0];
`endif // RANDOMIZE_REG_INIT
end // initial
`ifdef FIRRTL_AFTER_INITIAL
`FIRRTL_AFTER_INITIAL
`endif // FIRRTL_AFTER_INITIAL
`endif // ENABLE_INITIAL_REG_
assign o = reg_0;
endmodule
```

The module `Bar` does not have an asynchronous reset, while I would expect the contents to be identical to `Foo`.

The issue is that it only visits modules recursively through instance operations, starting at the top level module: https://github.com/llvm/circt/blob/f75bbd7986d4d5ab254ce428492f0e2366c16055/lib/Dialect/FIRRTL/Transforms/InferResets.cpp#L1459-L1467

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 in lib/Dialect/FIRRTL/Transforms/InferResets.cpp at the recursive module visitation around lines 1459-1467. Reproduce the issue with the provided FIRRTL input and inspect the generated Verilog. Done means FART applies the asynchronous reset annotation to Bar as well as Foo, including when Bar is not reachable through an instance operation.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 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.