[Mlir-commits] [mlir] [mlir][arith] Generalize ceil and floor division expansion (PR #206249)

[Hung-Kuan Tseng]

https://github.com/Tim096 commented:

I checked this locally on top of current main (it needs a small conflict resolution first, see 1.):

The formulas hold: an exhaustive i8 check of `q + (q*rhs != lhs)` and of the `slt`/`ult` same-sign test against Python's ceil/floor found no mismatch, and running the expanded IR through `mlir-runner` on 16 signed pairs (INT_MIN/INT_MAX with ±1, ±3, INT_MAX, INT_MIN, ...) and 16 unsigned pairs (UINT_MAX, 2^31, ...) matches as well. The existing `Integration/Dialect/Arith/CPU/test-arith-expand-ceildivsi.mlir` still passes, and the reproducer from #206091 no longer asserts under `-arith-expand` or `-gpu-to-llvm`.

Two things on the tests:

1. The PR conflicts with main in `arith-to-llvm.mlir`. Since #212074, `ceildivui %x, %x` folds to 1, and dialect conversion folds before it applies patterns, so after a rebase the `@ceildivui` test with `%arg0, %arg0` produces `llvm.mlir.constant(1 : i32)` instead of the `udiv` sequence. The test in main takes two operands now; keeping that shape fixes it.

2. The `VALUES` functions in `expand-ops.mlir` don't exercise the expansion: all operands are constants, so `arith-expand` folds `ceildivsi` & co. through the op folder before the pattern runs (`DialectConversionFoldingMode::BeforePatterns`). `mlir-opt -canonicalize` on main gives the same output. If you want value-level coverage, `test-arith-expand-ceildivsi.mlir` passes the operands through `func.call` so nothing folds; a `floordivsi`/`ceildivui` sibling in that style would cover the other two expansions, and the `@floordivsi_values` cases would fit there.


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