[mlir][arith] `--convert-arith-to-llvm` miscompiles `arith.ceildivsi`

[pingshiyu]

The following code computes ceildivsi -128, 7, with an expected result of -18.

However, --convert-arith-to-llvm produces an incorrect output of 18.

Reproduction: https://godbolt.org/z/cWf96T8v3

[Lewuathe]

Although I'm not sure which version (which trunk) the Godbelt is using, the latest main branch seems to return the expected value as follows.

./bin/mlir-opt test.mlir --convert-arith-to-llvm
module {
  func.func @main_graph() -> i32 {
    %0 = llvm.mlir.constant(-128 : i32) : i32
    %1 = llvm.mlir.constant(7 : i32) : i32
    %2 = llvm.mlir.constant(-18 : i32) : i32
    return %2 : i32
  }
}

commit : b86e099

[pingshiyu]

The bug is in general triggered by (slightly subtly) expressions ceildivsi MIN_INT / n

And so the i8 type and the -128 here are both required to see the bug.

Just wanted to check if the following exact file was used to test main? (Note the i8 type)

module {
  func.func @main() {
    %c7_i8 = arith.constant 7 : i8
    %c-128_i8 = arith.constant -128 : i8
    %0 = arith.ceildivsi %c-128_i8, %c7_i8 : i8
    vector.print %0 : i8
    return
  }
}

[banach-space]

Sorry, I missed this bug report when posting my solution:

• [mlir][arith] Fix overflow bug in arith::CeilDivSIOp::fold #90947

Tl;Dr, this bug is due to overflow flags not being checked by the folder. That's what my is PR trying to fix.

I know that @bviyer also posted a fix, but it's not clear to me whether that's the right approach:

• [arith][mlir] Fixed a bug in CeilDiv with neg values #90855
  See my comment here: [arith][mlir] Fixed a bug in CeilDiv with neg values #90855 (comment)

[banach-space]

the latest main branch seems to return the expected value as follows.

Not for me:

bin/mlir-opt --convert-arith-to-llvm file.mlir
module {
  func.func @main() {
    %0 = llvm.mlir.constant(7 : i8) : i8
    %1 = llvm.mlir.constant(-128 : i8) : i8
    %2 = llvm.mlir.constant(18 : i8) : i8
    vector.print %2 : i8
    return
  }
}

That's with 6d44a1e.

[Lewuathe]

Sorry, my code did not consider the type of the value (it used i32 instead). It was not reproducing the case in which the overflow happened. Your code is right.

#89382 (comment)