phanrahan/magma

[MLIR] Bug in always comb ordering

Closed this issue · 1 comments

class test_when_alwcomb_order_2(m.Circuit):
    io = m.IO(I=m.In(m.Bits[8]), S=m.In(m.Bits[2]), O=m.Out(m.Bits[8]))

    a = m.Register(m.Bits[8])(name="a")
    b = m.Register(m.Bits[8])(name="b")
    b.I @= a.O

    x = m.Bits[8]()
    x @= a.O

    with m.when(io.S[0]):
        x @= io.I
        b.I @= io.I

    with m.when(io.S[1]):
        a.I @= 0
    with m.otherwise():
        a.I @= x

    io.O @= b

m.compile(
    "build/test_when_alwcomb_order_2",
    test_when_alwcomb_order_2,
    output="mlir-verilog"
)

produces

// Generated by CIRCT circtorg-0.0.0-1773-g7abbc4313
module test_when_alwcomb_order_2(
  input  [7:0] I,
  input  [1:0] S,
  input        CLK,
  output [7:0] O
);

  reg [7:0] _GEN;
  reg [7:0] a;
  reg [7:0] _GEN_0;
  reg [7:0] _GEN_1;
  always_comb begin
    if (S[1])
      _GEN = 8'h0;
    else
      _GEN = _GEN_0;
    _GEN_0 = a;
    _GEN_1 = a;
    if (S[0]) begin
      _GEN_0 = I;
      _GEN_1 = I;
    end
  end // always_comb
  reg [7:0] b;
  always_ff @(posedge CLK) begin
    a <= _GEN;
    b <= _GEN_1;
  end // always_ff @(posedge)
  initial begin
    a = 8'h0;
    b = 8'h0;
  end // initial
  assign O = b;
endmodule

Notice that _GEN_0 (x in the source python) is referenced before it is assigned later in the always comb block. The issue has to do with the ordering of module compilation, i.e. the dependencies between the when primitive instances isn't not reflected in the order of the module compilation.

MLIR for reference

module attributes {circt.loweringOptions = "locationInfoStyle=none"} {
    hw.module @test_when_alwcomb_order_2(%I: i8, %S: i2, %CLK: i1) -> (O: i8) {
        %0 = comb.extract %S from 0 : (i2) -> i1
        %1 = comb.extract %S from 1 : (i2) -> i1
        %2 = hw.constant 0 : i8
        %5 = sv.reg : !hw.inout<i8>
        %4 = sv.read_inout %5 : !hw.inout<i8>
        sv.alwayscomb {
            sv.if %1 {
                sv.bpassign %5, %2 : i8
            } else {
                sv.bpassign %5, %3 : i8
            }
        }
        %7 = sv.reg name "a" : !hw.inout<i8>
        sv.alwaysff(posedge %CLK) {
            sv.passign %7, %4 : i8
        }
        %8 = hw.constant 0 : i8
        sv.initial {
            sv.bpassign %7, %8 : i8
        }
        %6 = sv.read_inout %7 : !hw.inout<i8>
        %10 = sv.reg : !hw.inout<i8>
        %3 = sv.read_inout %10 : !hw.inout<i8>
        %11 = sv.reg : !hw.inout<i8>
        %9 = sv.read_inout %11 : !hw.inout<i8>
        sv.alwayscomb {
            sv.bpassign %10, %6 : i8
            sv.bpassign %11, %6 : i8
            sv.if %0 {
                sv.bpassign %10, %I : i8
                sv.bpassign %11, %I : i8
            }
        }
        %13 = sv.reg name "b" : !hw.inout<i8>
        sv.alwaysff(posedge %CLK) {
            sv.passign %13, %9 : i8
        }
        sv.initial {
            sv.bpassign %13, %8 : i8
        }
        %12 = sv.read_inout %13 : !hw.inout<i8>
        hw.output %12 : i8
    }
}

After further investigation, I think the issue here is that there is a cycle in the instance graph. Now, this is not an issue for the standard MLIR backend given a normal graph representation and the cycles being broken by a register, but since MLIR "fuses" the always comb logic, an ordering issue does occur for the when code (this wouldn't be a problem if the always comb blocks were left separate).