llvm / circt

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

[InferResets] Not great error message for uninferred reset port #4183

Closed jackkoenig closed 1 year ago

jackkoenig commented 1 year ago

Consider:

circuit Top :
  extmodule Foo :
    input reset : Reset @[FizzBuzz.scala 123:10]
    input in : UInt<8>
    output out : UInt<8>

  module Top:
    input in : UInt<8>
    output out : UInt<8>

    inst foo of Foo
    foo.in <= in
    out <= foo.out

Run with firtool this gives the following error:

reset.fir:5:13: error: 'firrtl.extmodule' op contains an abstract reset type after InferResets
  extmodule Foo :
            ^
reset.fir:5:13: note: see current operation: 
"firrtl.extmodule"() ({
}) {annotations = [], parameters = [], portAnnotations = [], portDirections = -4 : i3, portNames = ["reset", "in", "out"], portSyms = [], portTypes = [!firrtl.reset, !firrtl.uint<8>, !firrtl.uint<8>], sym_name = "Foo", sym_visibility = "private"} : () -> ()

Not the best error, it would be ideal if it would tell me which port and to use the source locator (if available). Same thing seems to apply to ports of regular modules too.

jackkoenig commented 1 year ago

A recent bugfix in Chisel is going to expose a lot errors like this once it's released in Chisel 3.5.5.

seldridge commented 1 year ago

We can improve this. One thing that makes the error message worse than it could be is that we don't have source locators for ports. This would likely require a portSourceLocators attribute array on the module. However, I don't think Chisel provides source locators for ports currently. Is this something that could be added?

seldridge commented 1 year ago

New output:

issues/4183.fir:2:13: error: 'firrtl.extmodule' op has an abstract reset type port "reset" after InferResets
  extmodule Foo :
            ^
issues/4183.fir:2:13: note: see current operation: 
"firrtl.extmodule"() ({
}) {annotations = [], parameters = [], portAnnotations = [], portDirections = -4 : i3, portNames = ["reset", "in", "out"], portSyms = [], portTypes = [!firrtl.reset, !firrtl.uint<8>, !firrtl.uint<8>], sym_name = "Foo", sym_visibility = "private"} : () -> ()