[Mlir-commits] [mlir] [mlir][arith] Fold addi(x, not(x)) -> -1 (PR #212272)

[Victor Perez]

victor-eds wrote:

> I'd leave this for canonicalize only. But if anyone has a different opinion, I don't mind to add this for fold as well. @matthias-springer wdyt?

I looked into this a bit more. The hoists are sound:
- `addi(not(x), y) -> not(subi(x, y))`: https://alive2.llvm.org/ce/z/_p6M69
- `addi(x, not(y)) -> not(subi(y, x))`: https://alive2.llvm.org/ce/z/uVJeKz
- `subi(not(x), y) -> not(addi(x, y))` (`i16`; `i32` times out): https://alive2.llvm.org/ce/z/FTkKxJ

But LLVM itself doesn't perform them. Here's InstCombine on the same inputs: https://godbolt.org/z/Pv99Trfrb: it leaves `~x + y`, `x + ~y`, and `~x - y` as-is (only commuting operands), and for the constant cases it goes the opposite direction, folding the not into the constant (`~x + 7 -> 6 - x`, `~x - 5 -> -6 - x`, `9 - ~x -> x + 10`).

I think there are good reasons LLVM avoids the hoist:
- It's not a simplification — `~x + y` and `~(x - y)` are both two ops, so nothing is saved. Applicability would be narrowed to patterns like the ones I'm covering here.
- It's the "wrong canonical direction": it buries the `add`/`sub` inside a `not`, hiding it from the many patterns that match `add`/`sub`. LLVM's canonical form keeps the arithmetic visible and instead folds nots into constants when possible as in the examples I show in the link above.

So my suggestion is to keep the `x + ~x -> -1` fast-path fold here, that mirrors LLVM's InstructionSimplify one-to-one, and **skip** the general hoist. If we'd like a canonicalization in this area that does match LLVM, the not-into-constant folds `(~x + c -> (c-1) - x`, and the `~x - c / c - ~x` variants) are a genuine gap in arith, and I'm happy to add those as a follow-up.

WDYT?

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