llvm / circt

Circuit IR Compilers and Tools
https://circt.org
Other
1.67k stars 296 forks source link

[FIRRTL] Invalid Async Reset False Positive with DontTouch #3042

Closed seldridge closed 2 years ago

seldridge commented 2 years ago

If a constant wire is marked don't touch and used as the reset initial value, then CIRCT will complain that this is a "non-constant async reset value" during LowerToHW because it relies on IMCP to prove reset value constant-ness and IMCP will correctly not propagate through don't touch.

Consider the following circuit:

circuit ConstantReset:
  module ConstantReset:
    input clock: Clock
    input reset: AsyncReset
    input d: UInt<1>
    output q: UInt<1>
    input x: UInt<1>

    wire w: UInt<1>
    w <= UInt<1>(0)

    reg r: UInt<1>, clock with: (reset => (reset, w))

    r <= d
    q <= r

And the following annotation file:

[
  {
  "class":"firrtl.transforms.DontTouchAnnotation",
  "target":"~ConstantReset|ConstantReset>w"
  }
]

When compiled with firtool ConstantReset.fir everything works. If compiled with firtool ConstantReset.fir --annotation-file ConstantReset.anno.json, then this will error out:

ConstantReset.fir:12:5: error: register with async reset requires constant reset value
    reg r: UInt<1>, clock with: (reset => (reset, w))
    ^
ConstantReset.fir:12:5: note: see current operation: %12 = "firrtl.regreset"(%1, %2, %8) {annotations = [], name = "r"} : (!firrtl.clock, !firrtl.asyncreset, !firrtl.uint<1>) -> !firrtl.uint<1>
ConstantReset.fir:9:5: note: reset value defined here:
    wire w: UInt<1>
    ^

SFC will compile wither of these just fine.

As much as it pains me, this likely motivates moving the exact SFC CheckResets logic into RemoveInvalid and turning this into the omnibus SFCExactness pass.

seldridge commented 2 years ago

Fixed via https://github.com/llvm/circt/pull/3049.