[Mlir-commits] [mlir] [arith][mlir] Fixed a bug in CeilDiv with neg values (PR #90855)

[Andrzej Warzyński]

banach-space wrote:

This PR is missing some analysis explaining what the actual issue is. Without that, the proposed update seems rather arbitrary  - it's hard for me to see _why_ the proposed update fixes the issue if we don't know what "the issue" is.

> When either the divisor or dividend, not both, is negative the compiler gives a positive value, while it should be a negative one. 

As I hinted in my message above, the current logic _does work_ for negative values. It breaks when using MININT (e.g. -128` for `int8`). The solution should be built around that observation.

I've spent some time analysing this and found that overflow flags are effectively ignored. I felt like the easiest way to demonstrate/document/explain this was to post a patch:
* https://github.com/llvm/llvm-project/pull/90947

Now, I agree that the folder could still work for the case that you are testing:
```mlir
func.func @simple_arith.ceildivsi_i8() -> (i8) {
  %0 = arith.constant 7 : i8
  %1 = arith.constant -128 : i8
  %2 = arith.ceildivsi %1, %0 : i8
  return %2 : i8
}
``` 

However, first I'd make sure that all overflow flags are checked (and that no folding happens if there's an overflow). Following that, we can iterate and extend the folder to support your case. WDYT?

https://github.com/llvm/llvm-project/pull/90855