llvm / circt

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

[FIRRTL][LowerToHW] Zero-width signals with inner symbol are silently deleted, breaking users #5590

Open dtzSiFive opened 1 year ago

dtzSiFive commented 1 year ago

Consider this MLIR example:

module {
  firrtl.circuit "OMIRField" {
    firrtl.module @OMIRField(in %x_b: !firrtl.uint<0>, out %y_b: !firrtl.uint<0>) {
      %n_b = firrtl.node sym @omir_sym %x_b : !firrtl.uint<0>
      firrtl.strictconnect %y_b, %n_b : !firrtl.uint<0>
    }
  }
  sv.verbatim "Testing {{0}}" {symbols = [#hw.innerNameRef<@OMIRField::@omir_sym>]}
}

Feeding through LowerToHW: circt-opt --lower-firrtl-to-hw produces:

module {
  sv.verbatim "Testing {{0}}" {symbols = [#hw.innerNameRef<@OMIRField::@omir_sym>]}
  hw.module @OMIRField() {
    hw.output
  }
}

And if run through firtool directly, produces:

omir_zw_field_issue.mlir:8:3: error: cannot get name for symbol #hw.innerNameRef<@OMIRField::@omir_sym>
  sv.verbatim "Testing {{0}}" {symbols = [#hw.innerNameRef<@OMIRField::@omir_sym>]}
  ^
omir_zw_field_issue.mlir:8:3: note: see current operation: "sv.verbatim"() {format_string = "Testing {{0}}", symbols = [#hw.innerNameRef<@OMIRField::@omir_sym>]} : () -> ()
// Generated by CIRCT 1.46.0g20230714_98d0bf5
Testing <INVALID>   // omir_zw_field_issue.mlir:8:3
module OMIRField(); // omir_zw_field_issue.mlir:3:5
endmodule

The non-failing error isn't great and should be fixed (cc #4770 for similar in adjacent code). Also, this should be caught by the verifier in HW but that support isn't in place yet (cc #3526).


Moving the symbol to a port produces an error, modified input:

module {
  firrtl.circuit "OMIRField" {
    firrtl.module @OMIRField(in %x_b: !firrtl.uint<0> sym @omir_sym, out %y_b: !firrtl.uint<0>) {
      %n_b = firrtl.node %x_b : !firrtl.uint<0>
      firrtl.strictconnect %y_b, %n_b : !firrtl.uint<0>
    }
  }
  sv.verbatim "Testing {{0}}" {symbols = [#hw.innerNameRef<@OMIRField::@omir_sym>]}
}

Error:

omir_zw_field_issue.mlir:3:33: error: zero width port "x_b" is referenced by name [#hw<innerSym@omir_sym>] (e.g. in an XMR) but must be removed
    firrtl.module @OMIRField(in %x_b: !firrtl.uint<0> sym @omir_sym, out %y_b: !firrtl.uint<0>) {
                                ^
dtzSiFive commented 1 year ago

Reachable from this OMIR-leveraging input:

circuit OMIRField: %[[
 { "class": "freechips.rocketchip.objectmodel.OMIRAnnotation",
   "nodes": [
     {
       "fields": [
         {
            "info": "",
            "name": "track",
            "value": "OMDontTouchedReferenceTarget:~OMIRField|OMIRField>n"
         }
       ],
       "id": "OMID:0",
       "info": "dummy"
     }
   ]
 }
]]
  module OMIRField:
    input x : {a: UInt<5>, b: UInt<0>}
    output nb : UInt
    node n = x
    nb <= n.b
dtzSiFive commented 5 months ago

Other declarations as well, such as registers. Here's one reachable from FIRRTL:

FIRRTL version 4.0.0
circuit OMIRZW:
  public module OMIRZW:
    input x : {a: UInt<5>, b: UInt<0>}
    input c : Clock
    input reset : UInt<1>
    output out : UInt<0>
    output r_path : Path

    regreset r : UInt, c, reset, UInt<0>(0)
    connect r, x.b

    connect out, r

    propassign r_path, path("OMReferenceTarget:~OMIRZW|OMIRZW>r")