[mlir][arith] `ceildivsi` wrong lowering

[pingshiyu]

The following code computes the value (ceildivsi -128 7), and should produce -18. However, the compiled code will output 18 on trunk (when run with mlir-cpu-runner ).

A similar bug was previously reported in #89382, but the fix addressed only the constant folder. It appears the lowering is incorrect as well.

https://godbolt.org/z/4KzG3chYG

[jgiron42]

Hi, This is my first time contributing to this project. I'd like to give it a shot. Can someone assign this issue to me?

[jgiron42]

there seems to be a second bug going on with ceildiv 127 -1.
I think this behavior is caused by the fact that both branches of the select expression (in: CeilDivSIOpConverter) are evaluated by mlir-cpu-runner and the first one overflow.
So I'm not sure if this should be considered as a real bug but changing x = (m > 0) ? -1 : 1 to x = (n > 0) ? -1 : 1 seems to fix it.
Should I include this in the PR?

[Lewuathe]

I tried to run the ceildiv 127 -1 with the latest main branch. c28b1a1

./bin/mlir-opt --arith-expand --test-lower-to-llvm test.mlir
module {
  llvm.func @printNewline()
  llvm.func @printI64(i64)
  llvm.func @ceildivsi_i8(%arg0: i8, %arg1: i8) {
    %0 = llvm.mlir.constant(1 : i8) : i8
    %1 = llvm.mlir.constant(0 : i8) : i8
    %2 = llvm.mlir.constant(-1 : i8) : i8
    %3 = llvm.icmp "sgt" %arg1, %1 : i8
    %4 = llvm.select %3, %2, %0 : i1, i8
    %5 = llvm.add %4, %arg0 : i8
    %6 = llvm.sdiv %5, %arg1  : i8
    %7 = llvm.add %6, %0 : i8
    %8 = llvm.sub %1, %arg0 : i8
    %9 = llvm.sdiv %8, %arg1  : i8
    %10 = llvm.sub %1, %9 : i8
    %11 = llvm.icmp "slt" %arg0, %1 : i8
    %12 = llvm.icmp "sgt" %arg0, %1 : i8
    %13 = llvm.icmp "slt" %arg1, %1 : i8
    %14 = llvm.and %11, %13  : i1
    %15 = llvm.and %12, %3  : i1
    %16 = llvm.or %14, %15  : i1
    %17 = llvm.select %16, %7, %10 : i1, i8
    %18 = llvm.sext %17 : i8 to i64
    llvm.call @printI64(%18) : (i64) -> ()
    llvm.call @printNewline() : () -> ()
    llvm.return
  }
  llvm.func @ceildivsi() {
    %0 = llvm.mlir.constant(-1 : i8) : i8
    %1 = llvm.mlir.constant(127 : i8) : i8
    llvm.call @ceildivsi_i8(%1, %0) : (i8, i8) -> ()
    llvm.return
  }
  llvm.func @entry() {
    llvm.call @ceildivsi() : () -> ()
    llvm.return
  }
}

The CeilDivSIOpConverted seems to expand the code. But the result seems to be good.

./bin/mlir-opt --arith-expand --test-lower-to-llvm test.mlir | ./bin/mlir-cpu-runner --entry-point-result=void -e entry -shared-libs="lib/libmlir_c_runner_utils.dylib,lib/libmlir_runner_utils.dylib"
-127

Is there anything wrong with this outcome?

[jgiron42]

weird, maybe this is only an issue with the linux mlir-cpu-runner because it crash with the code your provided.
anyway I will just push the fix on this issue and maybe investigate on this other issue later.

[math-fehr]

My understanding is that #133774 should fix the bug?