[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

In D101759#2971567 <https://reviews.llvm.org/D101759#2971567>, @efriedma wrote:

> errno handling for math library functions is a mess.  Currently, we don't model it properly; we just mark the calls "readnone" and hope for the best.  If you don't want to fix that, just check for readnone for now.

I think using `readnone` would work fine. It seems that clang marks math functions with that attribute when `-fno-math-errno` is in effect. To get the non-finite MASS lowerings at -O3 one would have to compile with both `-fapprox-func` and `-fno-math-errno`, which seems reasonable to me.

> I don't think we want to be querying function attributes or options here; afn plus enabling MASS should be enough.  The function attributes are the old mechanism; we just haven't completely migrated some parts of SelectionDAG yet.

I agree. I think the problem is that this patch is trying to decide on a global lowering strategy for `llvm.*` math intrinsics in `llvm/lib/Target/PowerPC/PPCISelLowering.cpp` but such global decision making does not go well with finer granularity of fast-math flags. My understanding is that the reason we need to handle //intrinsic// math functions later is because of strength-reduction transformations like `pow(x,0.5) --> sqrt(x)` that currently operate on intrinsic calls only. If we could apply those operations on things like `__xl_pow_finite` and produce calls to `__xl_sqrt_finite` then we wouldn't have this problem. Another possibility might be to have two versions of `PPCGenScalarMASSEntries` one that handles non-intrinsics and runs earlier, and another one that handles intrinsics after transformations likes `pow(x,0.5) --> sqrt(x)` are done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101759/new/

https://reviews.llvm.org/D101759