[Calyx] Lower Arith CmpFOp to Calyx

[jiahanxie353]

This patch tries to lowers arith::cmpf to Calyx. Relevant issue: #7821

One thing I'm still trying to figure out is that, taking ult as an example, I tried to create a CombGroup to evaluate std_compareFN.lt || std_compareFN.unordered.

My test case is:

module {
  func.func @main(%arg0 : f32) -> i1 {
    %0 = arith.constant 4.2 : f32
    %1 = arith.cmpf ult, %arg0, %0 : f32

    return %1 : i1
  }
}

and I dump intermediate result after BuildOpGroups, it shows:

"calyx.component"() <{function_type = (i32, i1, i1, i1, i1, i1) -> (), portAttributes = [{}, {clk}, {reset}, {go}, {}, {done}], portDirections = -16 : i6, portNames = ["in0", "clk", "reset", "go", "out0", "done"]}> ({
^bb0(%arg0: i32, %arg1: i1, %arg2: i1, %arg3: i1, %arg4: i1, %arg5: i1):
  %0 = "hw.constant"() <{value = true}> : () -> i1
  %1 = "hw.constant"() <{value = true}> : () -> i1
  %2:6 = "calyx.register"() <{sym_name = "cmpf_0_reg"}> : () -> (i1, i1, i1, i1, i1, i1)
  %3:3 = "calyx.std_or"() <{sym_name = "std_or_0"}> : () -> (i1, i1, i1)
  %4 = "hw.constant"() <{value = true}> : () -> i1
  %5:12 = "calyx.ieee754.compare"() <{sym_name = "std_compareFN_0"}> : () -> (i1, i1, i1, i32, i32, i1, i1, i1, i1, i1, i5, i1)
  %6:6 = "calyx.register"() <{sym_name = "ret_arg0_reg"}> : () -> (i1, i1, i1, i1, i1, i1)
  %7 = "calyx.constant"() <{sym_name = "cst_0", value = 4.200000e+00 : f32}> : () -> i32
  "calyx.wires"() ({
    "calyx.assign"(%arg4, %6#4) : (i1, i1) -> ()
    "calyx.comb_group"() <{sym_name = "compute_compare_0_out"}> ({
      "calyx.assign"(%3#0, %5#9) : (i1, i1) -> ()
      "calyx.assign"(%3#1, %5#6) : (i1, i1) -> ()
    }) : () -> ()
    "calyx.group"() <{sym_name = "bb0_0"}> ({
      "calyx.assign"(%5#3, %arg0) : (i32, i32) -> ()
      "calyx.assign"(%5#4, %7) : (i32, i32) -> ()
      "calyx.assign"(%2#0, %3#2) : (i1, i1) -> ()
      "calyx.assign"(%2#1, %5#11) : (i1, i1) -> ()
      %8 = "hw.constant"() <{value = true}> : () -> i1
      %9 = "comb.xor"(%5#11, %8) : (i1, i1) -> i1
      "calyx.assign"(%5#2, %1, %9) : (i1, i1, i1) -> ()
      "calyx.group_done"(%2#5) : (i1) -> ()
    }) : () -> ()
    "calyx.group"() <{sym_name = "ret_assign_0"}> ({
      "calyx.assign"(%6#0, %3#2) : (i1, i1) -> ()
      "calyx.assign"(%6#1, %0) : (i1, i1) -> ()
      "calyx.group_done"(%6#5) : (i1) -> ()
    }) : () -> ()
  }) : () -> ()
  "calyx.control"() ({
  ^bb0:
  }) : () -> ()
}) {sym_name = "main", toplevel} : () -> ()

so clearly we can see:

    "calyx.comb_group"() <{sym_name = "compute_compare_0_out"}> ({
      "calyx.assign"(%3#0, %5#9) : (i1, i1) -> ()
      "calyx.assign"(%3#1, %5#6) : (i1, i1) -> ()
    }) : () -> ()

But after the entire execution, it finishes with:

module attributes {calyx.entrypoint = "main"} {
  calyx.component @main(%in0: i32, %clk: i1 {clk}, %reset: i1 {reset}, %go: i1 {go}) -> (%out0: i1, %done: i1 {done}) {
    %cst = calyx.constant @cst_0 <4.200000e+00 : f32> : i32
    %true = hw.constant true
    %cmpf_0_reg.in, %cmpf_0_reg.write_en, %cmpf_0_reg.clk, %cmpf_0_reg.reset, %cmpf_0_reg.out, %cmpf_0_reg.done = calyx.register @cmpf_0_reg : i1, i1, i1, i1, i1, i1
    %std_or_0.left, %std_or_0.right, %std_or_0.out = calyx.std_or @std_or_0 : i1, i1, i1
    %std_compareFN_0.clk, %std_compareFN_0.reset, %std_compareFN_0.go, %std_compareFN_0.left, %std_compareFN_0.right, %std_compareFN_0.signaling, %std_compareFN_0.lt, %std_compareFN_0.eq, %std_compareFN_0.gt, %std_compareFN_0.unordered, %std_compareFN_0.exceptionalFlags, %std_compareFN_0.done = calyx.ieee754.compare @std_compareFN_0 : i1, i1, i1, i32, i32, i1, i1, i1, i1, i1, i5, i1
    %ret_arg0_reg.in, %ret_arg0_reg.write_en, %ret_arg0_reg.clk, %ret_arg0_reg.reset, %ret_arg0_reg.out, %ret_arg0_reg.done = calyx.register @ret_arg0_reg : i1, i1, i1, i1, i1, i1
    calyx.wires {
      calyx.assign %out0 = %ret_arg0_reg.out : i1
      calyx.group @bb0_0 {
        calyx.assign %std_compareFN_0.left = %in0 : i32
        calyx.assign %std_compareFN_0.right = %cst : i32
        calyx.assign %cmpf_0_reg.in = %std_or_0.out : i1
        calyx.assign %cmpf_0_reg.write_en = %std_compareFN_0.done : i1
        %0 = comb.xor %std_compareFN_0.done, %true : i1
        calyx.assign %std_compareFN_0.go = %0 ? %true : i1
        calyx.group_done %cmpf_0_reg.done : i1
      }
      calyx.group @ret_assign_0 {
        calyx.assign %ret_arg0_reg.in = %std_or_0.out : i1
        calyx.assign %ret_arg0_reg.write_en = %true : i1
        calyx.group_done %ret_arg0_reg.done : i1
      }
    }
    calyx.control {
      calyx.seq {
          calyx.enable @bb0_0
          calyx.enable @ret_assign_0
      }
    }
  } {toplevel}
}

There's no appearance of that combinational group. Any insight why could this happen? @cgyurgyik @andrewb1999 @rachitnigam

[cgyurgyik]

My first guess is this is being inlined because of InlineCombGroups.

[jiahanxie353]

My first guess is this is being inlined because of InlineCombGroups.

Makes sense, and I think I shouldn’t put them into comb groups in the first place since std_compareFN.lt/unordered are sequential. And does that mean I should make registers to hold std_compareFN.lt/unordered values?

[cgyurgyik]

Makes sense, and I think I shouldn’t put them into comb groups in the first place since std_compareFN.lt/unordered are sequential. And does that mean I should make registers to hold std_compareFN.lt/unordered values?

Yes, if you want these to be read in sequential order then they need to be placed in registers.

[jiahanxie353]

Maybe I should refactor with buildLibraryBinaryPipeOp, it's tricky though

[cgyurgyik]

A few smaller comments.

[cgyurgyik]

What is SPECIAL? It isn't immediatley clear from your documentation.

[jiahanxie353]

changed it to noComb (hopefully the intention that it stands for "no combinational logic" is more obvious)

[jiahanxie353]

thanks @cgyurgyik ! I've made changes based on your review

[cgyurgyik]

Suggestion: It might even be simpler to say None, i.e., CombLogic::None seems like a less verbose version of CombLogic::noComb. Usually the NONE case is placed first as well, since it corresponds to the integer 0.

[cgyurgyik]

should there be a break here in case a default / another case is added?

[cgyurgyik]

Let's create a new file float_compare.mlir or similar containing these tests.

[cgyurgyik]

Nice, looking much better; I've added a few smaller comments. I am curious if you've verified at all the output of this code besides manually checking it, e.g., have you run the emitted code through the native Calyx compiler and checked the outputs via some backend like Icarus or SysVerilog?