[FIRRTL] InferResets FullReset (formerly FART) does not add resets to not reset fields of partially reset registers
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
The original iteration of this bug was fixed in https://github.com/llvm/circt/pull/6912, but it appears that fix only works for registers in modules contain the wire or port marked with a `circt.FullResetAnnotation`, but does not work for children of the module with the annotated port or wire.
Consider the following FIRRTL:
```firrtl
FIRRTL version 3.3.0
; CHECK-LABEL: module test(
circuit top :%[[{
"class":"circt.FullResetAnnotation",
"target":"~top|top>reset",
"resetType":"async"
}]]
module test :
input clock : Clock
input reset : AsyncReset
input in : { foo : UInt<8>, bar : UInt<8>}
output out : { foo : UInt<8>, bar : UInt<8>}
wire reg1_w : { foo : UInt<8>, bar : UInt<8>}
invalidate reg1_w
; CHECK: reg1_foo <= 8'hC;
; CHECK: reg1_bar <= 8'h0;
connect reg1_w.foo, UInt<8>(0hc)
invalidate reg1_w.bar
; CHECK: reg1_foo = 8'hC;
; CHECK: reg1_bar = 8'h0;
regreset reg1 : { foo : UInt<8>, bar : UInt<8>}, clock, reset, reg1_w
wire reg2 : { foo : UInt<8>, bar : UInt<8>}
connect reg1, in
connect reg2, reg1
connect out, reg2
module top :
input clock : Clock
input reset : AsyncReset
input in : { foo : UInt<8>, bar : UInt<8>}
output out : { foo : UInt<8>, bar : UInt<8>}
inst child of test
connect child.clock, clock
connect child.reset, reset
connect child.in, in
connect out, child.out
```
Note that this is exactly the same as the first test in https://github.com/llvm/circt/blob/f75bbd7986d4d5ab254ce428492f0e2366c16055/test/firtool/async-reset.fir#L5, except that there is a new top module wrapping the module with registers, and the annotated port is in the top module.
Currently, `reg1.bar` does not properly get the expected async reset to `0`, but it should. If you change the annotation to target `~top|test>reset`, you will see the correct output.
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 test/firtool/async-reset.fir and its first test, then compare it with the reproducer's added top wrapper and FullResetAnnotation target. Confirm the InferResets FullReset behavior when the annotation is on the parent module: reg1.bar should receive the expected asynchronous reset to 0.
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