[Mlir-commits] [mlir] [mlir][math]Update `convertPowfOp` `ExpandPatterns.cpp` (PR #124402)

[Benoit Jacob]

================
@@ -311,40 +311,40 @@ static LogicalResult convertFPowIOp(math::FPowIOp op,
   return success();
 }
 
-// Converts  Powf(float a, float b) (meaning a^b) to exp^(b * ln(a))
+// Converts Powf(float a, float b) (meaning a^b) to exp^(b * ln(|a|))
+// * sign(a)^b
----------------
bjacob wrote:

I expected `math.powf(a, b)` to be restricted to the case of `a > 0`. It is not currently mentioned in the documentation, but I believe that that is an omission.

The formula in this comment, `a ^ b = e^(b * ln |a|) * sign(a)^b`, is applicable in two separate cases:
1. It is correct whenever `a > 0`, in which case it boils down to the usual  `a ^ b = e^(b * ln |a|)`.
2. It is correct when `b` is an integer, in which case `sign(a)^b` is well-defined even when `sign(a) == -1`.

Outside of the above cases, that is, when `a < 0` and `b` is not integral, the problem is that `a^b` would have to be a non-real complex number, and an additional would be that it would be determined only up to a +/- 1 factor. For example, when `a = -1` and `b = 0.5`, then `a^b` would be `+/- i`.

The code in this PR is actually implementing something different. It has to, since it can't produce complex numbers; that makes the above comment an imperfect description of what the code actually does.

What the code in this PR does is that it evaluates `remf(b, 2.0)`. When that returns exactly `remf(b, 2.0) == 0.0`, it returns just `a ^ b = e^(b * ln |a|)`. Otherwise, when `remf(b, 2.0) != 0.0`, it returns `a ^ b = - e^(b * ln |a|)`.

The problems with that are:
1. This is unreliable for large values of `b`. First because `remf` may be inexact, and then, for even larger values of `b` or for narrower floating-point types, because when `b` is meant to encode a large integer, it may be rounded to the nearest representable value which may not be exactly integral. In either case, `remf(b, 2.0)` may fail to be exactly `0.0` in cases where it was expected to be.
2. `remf` is expensive, so this attempt at supporting `a < 0` is a cost on all users of `math.powf`. The branch-less implementation implies that the cost is paid even when `a > 0` even though the result of the computation is effectively unused in that case.

Summary: My recommendation: restrict `math.powf` to the case where `a > 0`. Correspondingly simplify the docs, the above comment, and the lowering. The `remf` goes away.

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