[Moore] Input triggers assertion in canonicalizer infra

[maerhart]

circt-opt -canonicalize triggers an assertion on the following input. Maybe an upstream bug?

Assertion failed: (mayBeGraphRegion(*op->getParentRegion()) && "expected that op has no uses"), function operator(), file PatternMatch.cpp, line 182.

module {
  moore.module private @snitch_regfile(in %clk_i : !moore.l1, in %raddr_i : !moore.array<2 x l5>, out rdata_o : !moore.array<2 x l32>, in %waddr_i : !moore.array<1 x l5>, in %wdata_i : !moore.array<1 x l32>, in %we_i : !moore.l1) {
    %0 = moore.constant 1 : i32
    %1 = moore.constant 0 : i32
    %rdata_o = moore.variable : <array<2 x l32>>
    moore.procedure always_ff {
      cf.br ^bb1(%1 : !moore.i32)
    ^bb1(%3: !moore.i32):  // 2 preds: ^bb0, ^bb6
      moore.return
    ^bb2:  // no predecessors
      cf.br ^bb4
    ^bb3:  // no predecessors
      cf.br ^bb4
    ^bb4:  // 2 preds: ^bb2, ^bb3
      %4 = moore.add %4, %0 : i32
      cf.br ^bb6
    ^bb5:  // no predecessors
      cf.br ^bb6
    ^bb6:  // 2 preds: ^bb4, ^bb5
      %5 = moore.add %3, %0 : i32
      cf.br ^bb1(%5 : !moore.i32)
    }
    %2 = moore.read %rdata_o : <array<2 x l32>>
    moore.output %2 : !moore.array<2 x l32>
  }
}

[mingzheTerapines]

@hailongSun2000 let's have a look.

[terapines-osc-circt]

Hey, @maerhart. After I modify the code snippet from

 ^bb4:  // 2 preds: ^bb2, ^bb3
      %4 = moore.add %4, %0 : i32
      cf.br ^bb6

to

 ^bb4:  // 2 preds: ^bb2, ^bb3
      %4 = moore.add %3, %0 : i32
      cf.br ^bb6

It can work.

[terapines-osc-circt]

For the Snitch RISC-V Core, does it will generate %4 = moore.add %4, %0 🤔?

[maerhart]

Not directly, I used circt-reduce to get a smaller test case that still shows the same issue.
It's interesting that there's a combinational cycle inside a procedure.

There are a few things that we might want to investigate:

• Why is the comb cycle in the procedure not already rejected by the verifier after parsing (with a proper error message)?
• Is this comb cycle also present in the non-reduced version of snitch? If yes, can this only occur in unreachable blocks? If no, is there a pass in the pipeline that has a bug?
• Check which pass leaves these basic blocks without predecessors in the IR and make sure it doesn't anymore.

[terapines-osc-circt]

Your example is so special 🧐!
As long as cf.br can jump the block containing the comb cycle, MLIR throws a proper error message for the comb cycle. For example:

hw.module private @Top(){
  %0 = hw.constant 1 : i32
  llhd.process {
    ^bb1:
      cf.br ^bb2
    ^bb2:
      %1 = comb.add %1, %0 : i32
      llhd.halt
  }
  hw.output
}

test08.mlir:7:12: error: operand #0 does not dominate this use
      %1 = comb.add %1, %0 : i32
           ^
test08.mlir:7:12: note: see current operation: %1 = "comb.add"(%1, %0) : (i32, i32) -> i32
test08.mlir:7:12: note: operand defined here (op in the same block)

I use the circt-verilog tool to run the relevant modules(snitch_regfile) in different .sv files. No error is thrown.
I failed to use circt-reduce, but I noticed the --include=<string> and --test=<string> options. Please teach me how to use it(Thanks in advance 😃!), and then I can reproduce this error.

[terapines-osc-circt]

If I understand correctly, due to cf being SSACFG, and the comb cycle like %4 = moore.add %4, %0, the %4 is used(user/use) by itself, so MLIR detects this IR & block which has no predecessors, then the following error is thrown.

How should we suppress this stack dump and substitute it with an error message 🤔? Or avoid SSACFG containing the comb cycle? WDYT?

[maerhart]

I think the first thing we should do is find the pass in the circt-verilog pipeline that leaves behind these blocks with no predecessors and make sure that it removes them. They are causing issues at several places, so fixing it at the root is probably the easiest solution for now.