[all-commits] [llvm/llvm-project] b6ab04: [mlir][arith] Fix arith maxnumf/minnumf folder (#1...

[Clément Fournier via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: b6ab04c69c907362dc7ab65eb43a9907c9adcdc1
      https://github.com/llvm/llvm-project/commit/b6ab04c69c907362dc7ab65eb43a9907c9adcdc1
  Author: Clément Fournier <clem.fournier at proton.me>
  Date:   2024-11-27 (Wed, 27 Nov 2024)

  Changed paths:
    M mlir/include/mlir/IR/Matchers.h
    M mlir/lib/Dialect/Arith/IR/ArithOps.cpp
    M mlir/test/Dialect/Arith/canonicalize.mlir

  Log Message:
  -----------
  [mlir][arith] Fix arith maxnumf/minnumf folder (#114595)

Fix #114594 
#### Context

[IEEE754-2019](https://ieeexplore.ieee.org/document/8766229) Sec 9.6
defines 2 minimum and 2 maximum operations. They are termed
- `maximum` and `maximumNumber`
- `minimum` and `minimumNumber`

In the arith dialect they are respectively named `maximumf` and
`maxnumf`, `minimumf` and `minnumf` so I use these names.

These operations only differ in how they handle NaN values. For
`maximumf` and `minimumf`, if any operand is NaN, then the result is
NaN, ie, NaN is propagated. For `maxnumf` and `minnumf`, if any operand
is NaN, then the other operand is returned, ie, NaN is absorbed. The
following identities hold:
```
maximumf(x, NaN) = maximumf(NaN, x) = NaN
maxnumf(x, NaN) = maxnumf(NaN, x) = x
```
(and same for min).

#### Arith folders

In the following I am talking about the folders for the arith
operations. The folders implement the following canonicalizations (`op`
is one of maximumf, maxnumf, minimumf, minnumf):
1. `op(x, x)` folds to `x` 
2. for `op(x, y)`, if `y` folds to the neutral element of the `op`, then
the `op` is folded to `x`.
    1. The neutral element of `maximumf` is -Infty
    2. The neutral element of `minimumf` is +Infty
3. The neutral element of `maxnumf` and `minnumf` is NaN as shown above.
3. for `op(x, y)`, if both `x` and `y` fold to constants `x'` and `y'`,
then the `op` is folded and the result is calculated with a
corresponding runtime function.

The folders are properly implemented for `maximumf` and `minimumf`, but
the same implementations were copied for the respective `maxnumf` and
`minnumf` functions. This means the neutral element of the second folder
above is wrong:
- `maxnumf(x, -Infty)` is folded to `x`, but that's wrong, because if
`x` is NaN then -Infty should be the result
- `minnumf(x, +Infty)` is folded to `x`, but same thing, the result
should be +Infty when `x` is NaN.

This is fixed by using `NaN` as neutral element for the `maxnumf` and
`minnumf` ops.[^1]

Again because of copy paste mistake, the third pattern above is using
`llvm::maximum` instead of `llvm::maximumnum` to calculate the result in
case both arguments fold to a constant:
- `maxnumf(NaN, x')` would have been folded to `llvm::maximum(NaN, x')`
which is `NaN`, whereas the result should be `x'`.

This folder for `minnumf` already correctly uses `llvm::minnum`, but I
fixed the one for `maxnumf` in this PR.


[^1]: this is by the way already correctly implemented in
[`arith::getIdentityValueAttr`](https://github.com/oowekyala/llvm-project/blob/a821964e0320d1e35514ced149ec10ec06d7131a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp#L2493-L2498)



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications