Sanjay Patel via llvm-dev
2021-Sep-02 17:58 UTC
[llvm-dev] Do we need intrinsics for floating-point classification functions?
Thank you for restarting this proposal. It's a bit more work, but let's make sure we have a good design, so we don't miss any details. What optimizations/analysis does this intrinsic enable **in IR** in strict-FP mode? If it is only constant folding, we don't need to add an intrinsic. We fold lots of math library calls with no corresponding LLVM intrinsic under llvm::ConstantFoldCall(). We also do transforms based on operands/uses of library calls in SimplifyLibCalls. So I'm really asking: why do we translate isnan() in clang into something else in the first place? The backend can lower that call into whatever suits the target without involving LLVM IR. On Thu, Sep 2, 2021 at 8:33 AM Serge Pavlov via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Hi all, > > Some time ago a new intrinsic `llvm.isnan` was introduced, which was > intended to represent IEEE-754 operation `isNaN` as well as a family of C > library functions `isnan*`. Then a concern was raised (see > https://reviews.llvm.org/D104854) that this functionality should be > removed. Discussion in the subsequent RFC ( > https://lists.llvm.org/pipermail/llvm-dev/2021-August/152257.html) came > to consensus that such intrinsic is necessary. Nevertheless the patches > related to the new intrinsic were reverted. I have to restart the > discussion in hope to convince the community that this intrinsic and other > classification functions are necessary. > > There are two main reasons why this intrinsic is necessary: > 1. It allows correct implementation of `isnan` if strict floating point > semantics is in effect, > 2. It allows preserving the check in -ffast-math compilation. > > To facilitate the discussion let's concentrate on the first problem. > > Previously the frontend intrinsic `__builtin_isnan` was converted into > `cmp uno` during IR generation in clang codegen. This solution is not > suitable if FP exceptions are not ignored, because compare instructions > raise exceptions if its argument is signaling NaN. Both IEEE-754 (5.7.2) an > C standard (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2596.pdf, > F.3p6) demand that this function does not raise floating point exceptions. > There was no target-independent IR construct that could represent `isnan`. > > This drawback was significant enough and some attempts to alleviate it > were undertaken. In https://reviews.llvm.org/D95948 `isnan` was > implemented using integer operations in strictfp functions. It however is > not suitable for targets where a more efficient way exists, like dedicated > instruction. Another solution was implemented in > https://reviews.llvm.org/D96568, where a hook > `clang::TargetCodeGenInfo::testFPKind` was introduced, which injects target > specific code into IR. Such a solution makes IR more target-dependent and > prevents some IR-level optimizations. > > To have a solution suitable for all cases, a new intrinsic function > `llvm.isnan` was introduced (https://reviews.llvm.org/D104854). It > protects the check from undesirable optimizations and preserves it till > selector, where it can be lowered in optimal for a particular target way. > > Other classification functions also need their own intrinsics. In strictfp > mode even a check for zero (`iszero`) cannot be made by comparing a value > against zero, - if the value is signaling NaN, FP exceptions would be > raised. James Y Knight in the previous discussion ( > https://lists.llvm.org/pipermail/llvm-dev/2021-August/152282.html) listed > such "non-computational" functions, which should not signal if provided > with an sNAN argument. > > It looks like new intrinsic is the only consistent and in target-agnostic > way to implement these checks in all environments including the case when > FP exceptions are not ignored. > > Any feedback is welcome. > > Thanks, > --Serge > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210902/75f4208e/attachment.html>
Serge Pavlov via llvm-dev
2021-Sep-02 18:34 UTC
[llvm-dev] Do we need intrinsics for floating-point classification functions?
On Fri, Sep 3, 2021 at 12:58 AM Sanjay Patel <spatel at rotateright.com> wrote:> Thank you for restarting this proposal. It's a bit more work, but let's > make sure we have a good design, so we don't miss any details. > > What optimizations/analysis does this intrinsic enable **in IR** in > strict-FP mode? > If it is only constant folding, we don't need to add an intrinsic. We fold > lots of math library calls with no corresponding LLVM intrinsic under > llvm::ConstantFoldCall(). > We also do transforms based on operands/uses of library calls in > SimplifyLibCalls. >Optimization is not the motivation for this intrinsic. If FP exceptions are ignored, unordered comparison can be used to implement `isnan`. If not, we need to make the test without affecting FP exceptions. Without this intrinsic there is no way to represent such a test.> So I'm really asking: why do we translate isnan() in clang into something > else in the first place? The backend can lower that call into whatever > suits the target without involving LLVM IR. >It must be a function, known to the backend without declarations/definitions, that is an intrinsic.> > On Thu, Sep 2, 2021 at 8:33 AM Serge Pavlov via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Hi all, >> >> Some time ago a new intrinsic `llvm.isnan` was introduced, which was >> intended to represent IEEE-754 operation `isNaN` as well as a family of C >> library functions `isnan*`. Then a concern was raised (see >> https://reviews.llvm.org/D104854) that this functionality should be >> removed. Discussion in the subsequent RFC ( >> https://lists.llvm.org/pipermail/llvm-dev/2021-August/152257.html) came >> to consensus that such intrinsic is necessary. Nevertheless the patches >> related to the new intrinsic were reverted. I have to restart the >> discussion in hope to convince the community that this intrinsic and other >> classification functions are necessary. >> >> There are two main reasons why this intrinsic is necessary: >> 1. It allows correct implementation of `isnan` if strict floating point >> semantics is in effect, >> 2. It allows preserving the check in -ffast-math compilation. >> >> To facilitate the discussion let's concentrate on the first problem. >> >> Previously the frontend intrinsic `__builtin_isnan` was converted into >> `cmp uno` during IR generation in clang codegen. This solution is not >> suitable if FP exceptions are not ignored, because compare instructions >> raise exceptions if its argument is signaling NaN. Both IEEE-754 (5.7.2) an >> C standard (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2596.pdf, >> F.3p6) demand that this function does not raise floating point exceptions. >> There was no target-independent IR construct that could represent `isnan`. >> >> This drawback was significant enough and some attempts to alleviate it >> were undertaken. In https://reviews.llvm.org/D95948 `isnan` was >> implemented using integer operations in strictfp functions. It however is >> not suitable for targets where a more efficient way exists, like dedicated >> instruction. Another solution was implemented in >> https://reviews.llvm.org/D96568, where a hook >> `clang::TargetCodeGenInfo::testFPKind` was introduced, which injects target >> specific code into IR. Such a solution makes IR more target-dependent and >> prevents some IR-level optimizations. >> >> To have a solution suitable for all cases, a new intrinsic function >> `llvm.isnan` was introduced (https://reviews.llvm.org/D104854). It >> protects the check from undesirable optimizations and preserves it till >> selector, where it can be lowered in optimal for a particular target way. >> >> Other classification functions also need their own intrinsics. In >> strictfp mode even a check for zero (`iszero`) cannot be made by comparing >> a value against zero, - if the value is signaling NaN, FP exceptions would >> be raised. James Y Knight in the previous discussion ( >> https://lists.llvm.org/pipermail/llvm-dev/2021-August/152282.html) >> listed such "non-computational" functions, which should not signal if >> provided with an sNAN argument. >> >> It looks like new intrinsic is the only consistent and in target-agnostic >> way to implement these checks in all environments including the case when >> FP exceptions are not ignored. >> >> Any feedback is welcome. >> >> Thanks, >> --Serge >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210903/4e925798/attachment.html>