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>
Sanjay Patel via llvm-dev
2021-Sep-02 19:07 UTC
[llvm-dev] Do we need intrinsics for floating-point classification functions?
On Thu, Sep 2, 2021 at 2:34 PM Serge Pavlov <sepavloff at gmail.com> wrote:> > 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. >Here's a way to represent such a test without an intrinsic: declare i1 @isnan(double) ... %r = call i1 isnan(double x) #1 ... attributes #1 = { nounwind } The IR optimizer shouldn't do anything with that call because it might read/write arbitrary memory - assuming I got the (lack of) function attributes correct; let me know if this should be spelled differently. We can guarantee that an arbitrary call will survive as-is through the entire IR optimizer pipeline since optimization is not the motivation. I think there's even proof that this already works the way you want because on the clang on macOS, I see: #include <math.h> int a_libcall(double x) { return isnan(x); } $ clang -O2 isnan.c -S -emit-llvm -o - -ffast-math define i32 @a_libcall(double %0) local_unnamed_addr #1 { %2 = tail call i32 @__isnand(double %0) #3 ret i32 %2 }> >> 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. >Here's a sqrt() mathlib call in IR that gets simultaneously lowered to both a target-specific instruction AND a libcall on x86: https://godbolt.org/z/89xETxK6h We can do something like that for isnan() - if the target has an instruction that implements the IEEE/C definition of isnan, use it. Otherwise, make a call to the mathlib.> >> >> 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/0d9846c9/attachment.html>