-convert-fsm-to-sv Pass Fail

Hi, everyone! I’m playing around with the FSM dialect. I found this code snippet in test/Conversion/FSMToSVtest_basics.mlir:

fsm.machine @FSM(%arg0: i1, %arg1: i1) -> (i8) attributes {initialState = "A"} {
  %c_0 = hw.constant 0 : i8
  fsm.state @A output  {
    fsm.output %c_0 : i8
  } transitions {
    fsm.transition @B
  }

  fsm.state @B output  {
    fsm.output %c_0 : i8
  } transitions {
    fsm.transition @A
  }
}

hw.module @top(%arg0: i1, %arg1: i1, %clk : i1, %rst : i1) -> (out: i8) {
    %out = fsm.hw_instance "fsm_inst" @FSM(%arg0, %arg1), clock %clk, reset %rst : (i1, i1) -> (i8)
    hw.output %out : i8
}

It can be converted into SV dialect like this:

module {
  sv.verbatim "`include \22fsm_enum_typedefs.sv\22"
  hw.type_scope @fsm_enum_typedecls {
    hw.typedecl @FSM_state_t : !hw.enum<A, B>
  } {output_file = #hw.output_file<"fsm_enum_typedefs.sv">}
  hw.module @FSM(%in0: i1, %in1: i1, %clk: i1, %rst: i1) -> (out0: i8) {
    %A = hw.enum.constant A : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
    %to_A = sv.reg sym @A : !hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>
    sv.assign %to_A, %A : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
    %0 = sv.read_inout %to_A : !hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>
    %B = hw.enum.constant B : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
    %to_B = sv.reg sym @B : !hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>
    sv.assign %to_B, %B : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
    %1 = sv.read_inout %to_B : !hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>
    %state_next = sv.reg : !hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>
    %2 = sv.read_inout %state_next : !hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>
    %state_reg = seq.compreg %2, %clk, %rst, %0  : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
    %c0_i8 = hw.constant 0 : i8
    %output_0 = sv.reg : !hw.inout<i8>
    sv.alwayscomb {
      sv.case %state_reg : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
      case A: {
        sv.bpassign %state_next, %1 : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
        sv.bpassign %output_0, %c0_i8 : i8
      }
      case B: {
        sv.bpassign %state_next, %0 : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
        sv.bpassign %output_0, %c0_i8 : i8
      }
      default: {
      }
    }
    %3 = sv.read_inout %output_0 : !hw.inout<i8>
    hw.output %3 : i8
  }
  hw.module @top(%arg0: i1, %arg1: i1, %clk: i1, %rst: i1) -> (out: i8) {
    %fsm_inst.out0 = hw.instance "fsm_inst" @FSM(in0: %arg0: i1, in1: %arg1: i1, clk: %clk: i1, rst: %rst: i1) -> (out0: i8)
    hw.output %fsm_inst.out0 : i8
  }
}

But the sv.case op caused an error:

sv.mlir:21:7: error: 'sv.case' op region #2 ('caseRegions') failed to verify constraint: region with 1 blocks
      sv.case %state_reg : !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>
      ^
sv.mlir:21:7: note: see current operation: 
"sv.case"(%8) ({
  "sv.bpassign"(%6, %5) : (!hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>, !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>) -> ()
  "sv.bpassign"(%10, %9) : (!hw.inout<i8>, i8) -> ()
}, {
  "sv.bpassign"(%6, %2) : (!hw.inout<typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>, !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>) -> ()
  "sv.bpassign"(%10, %9) : (!hw.inout<i8>, i8) -> ()
}, {
}) {casePatterns = [#hw.enum.field<A, !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>, #hw.enum.field<B, !hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>>, unit], caseStyle = 0 : i32, validationQualifier = #sv<validation_qualifier plain>} : (!hw.typealias<@fsm_enum_typedecls::@FSM_state_t, !hw.enum<A, B>>) -> ()

I’ve tried the pass with several codes written in fsm, they are have this problem. Is this because there is no default case after lowering? Then why this pass would generate this one at all? I commented the default region and then it’s good.

I’ve tried with other FSMs and I found out that the problem always occured when there is a empty region of case. But after the lowering, some cases are actually empty, maybe we need to check somehow to prune those so that the verifier of CaseOp won’t raise the error again?

If I simply add a bb0 in the empty cases or just comment them, the problem is gone, whis becomes a MLIR problem actually. So I wonder that’s because the bb0 is implicit and MLIR doesn’t know that, then I tried to print the generic form of the output mlir after -lower-fsm-to-sv, because if the pass works properly there must be bb0 in the “empty” case, then this new problem shows up, which should be a CIRCT problem:

hw_fsm.mlir:7:3: error: 'hw.module' op block argument locations should match signature locations
  "hw.module"() ({
  ^
hw_fsm.mlir:7:3: note: see current operation: 
"hw.module"() ({
^bb0(%arg0: i1, %arg1: i1, %arg2: i1):
  %0 = "hw.enum.constant"() {field = #hw.enum.field<IDLE, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>} : () -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %1 = "sv.reg"() {inner_sym = "IDLE", name = "to_IDLE"} : () -> !hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>
  "sv.assign"(%1, %0) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
  %2 = "sv.read_inout"(%1) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %3 = "hw.enum.constant"() {field = #hw.enum.field<BUSY, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>} : () -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %4 = "sv.reg"() {inner_sym = "BUSY", name = "to_BUSY"} : () -> !hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>
  "sv.assign"(%4, %3) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
  %5 = "sv.read_inout"(%4) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %6 = "sv.reg"() {name = "state_next"} : () -> !hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>
  %7 = "sv.read_inout"(%6) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %8 = "seq.compreg"(%7, %arg1, %arg2, %2) {name = "state_reg"} : (!hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>, i1, i1, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %9 = "sv.reg"() {name = "cnt_next"} : () -> !hw.inout<i16>
  %10 = "hw.constant"() {value = 0 : i16} : () -> i16
  %11 = "sv.read_inout"(%9) : (!hw.inout<i16>) -> i16
  %12 = "seq.compreg"(%11, %arg1, %arg2, %10) {name = "cnt_reg"} : (i16, i1, i1, i16) -> i16
  %13 = "hw.constant"() {value = 1 : i16} : () -> i16
  %14 = "hw.constant"() {value = 0 : i16} : () -> i16
  %15 = "hw.constant"() {value = false} : () -> i1
  %16 = "hw.constant"() {value = 256 : i16} : () -> i16
  %17 = "hw.constant"() {value = true} : () -> i1
  %18 = "comb.mux"(%arg0, %5, %2) : (i1, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %19 = "comb.sub"(%12, %13) : (i16, i16) -> i16
  %20 = "comb.icmp"(%12, %14) {predicate = 1 : i64} : (i16, i16) -> i1
  %21 = "comb.icmp"(%12, %14) {predicate = 0 : i64} : (i16, i16) -> i1
  %22 = "comb.mux"(%21, %2, %5) : (i1, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %23 = "comb.mux"(%20, %5, %22) : (i1, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>
  %24 = "sv.reg"() {name = "output_0"} : () -> !hw.inout<i1>
  "sv.alwayscomb"() ({
    "sv.bpassign"(%9, %12) : (!hw.inout<i16>, i16) -> ()
    "sv.case"(%8) ({
      "sv.bpassign"(%6, %18) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
      "sv.case"(%7) ({
      ^bb0:
      }, {
        "sv.bpassign"(%9, %16) : (!hw.inout<i16>, i16) -> ()
      }, {
      ^bb0:
      }) {casePatterns = [#hw.enum.field<IDLE, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, #hw.enum.field<BUSY, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, unit], caseStyle = 0 : i32, validationQualifier = #sv<validation_qualifier plain>} : (!hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
      "sv.bpassign"(%24, %17) : (!hw.inout<i1>, i1) -> ()
    }, {
      "sv.bpassign"(%6, %23) : (!hw.inout<typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
      "sv.case"(%7) ({
      ^bb0:
      }, {
        "sv.bpassign"(%9, %19) : (!hw.inout<i16>, i16) -> ()
      }, {
      ^bb0:
      }) {casePatterns = [#hw.enum.field<IDLE, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, #hw.enum.field<BUSY, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, unit], caseStyle = 0 : i32, validationQualifier = #sv<validation_qualifier plain>} : (!hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
      "sv.bpassign"(%24, %15) : (!hw.inout<i1>, i1) -> ()
    }, {
    ^bb0:
    }) {casePatterns = [#hw.enum.field<IDLE, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, #hw.enum.field<BUSY, !hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>>, unit], caseStyle = 0 : i32, validationQualifier = #sv<validation_qualifier plain>} : (!hw.typealias<@fsm_enum_typedecls::@foo_state_t, !hw.enum<IDLE, BUSY>>) -> ()
  }) : () -> ()
  %25 = "sv.read_inout"(%24) : (!hw.inout<i1>) -> i1
  "hw.output"(%25) : (i1) -> ()
}) {argLocs = [loc(unknown), loc(unknown), loc(unknown)], argNames = ["in0", "clk", "rst"], arg_attrs = [{}, {}, {}], comment = "", function_type = (i1, i1, i1) -> i1, parameters = [], res_attrs = [{}], resultLocs = [loc(unknown)], resultNames = ["out0"], sym_name = "foo"} : () -> ()

This is a bug in CIRCT. To summarize:

To the parser, an empty region is indistiguishable from a region containing the empty block. During round-tripping through text, the verifier of the sv dialect will fail on verifying SingleBlock for CaseOp. This can be manually fixed by adding ^bb0: to the affected regions. An ImplicitTerminator instead of a NoTerminator also does not suffer from that issue, because it will be implicitly inserted.

The basis for a possible fix is found in this discourse post. The affected line would be this one:

This is the function that is called there:

I propose passing the 3rd, optional argument as true to force empty blocks to be printed. I don’t know what the contribution process is here, so I’ll leave this up to you guys.

There also seems to be another issue. The above problem should not arise when printing the generic format using -mlir-print-generic-op. However, the generic output is invalid, and fails validation because of incorrectly attached locations (?). I have never heard of this before, but I suspect it is also a CIRCT issue.

I haven’t paged in what is going on in the FSM dialect, but it does seem like this should be fixed. Not sure if there is any open issue for this yet, but feel free to open an issue or just send a PR if you have a solution in mind.

For this issue, this is known: CIRCT cannot consume the generic format it generates · Issue #5484 · llvm/circt · GitHub.