llvm dev - Sep 2021 - [cfe-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>

On Thu, Sep 2, 2021 at 3:07 PM Sanjay Patel via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
> On Thu, Sep 2, 2021 at 2:34 PM Serge Pavlov <sepavloff at gmail.com>
wrote:
>
>> 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 }
>
No -- there is no "isnan" standard library call. C "isnan"
is a macro which
expands to whatever each libc desires. There is effectively no
standardization as to what library function -- if any -- they actually end
up calling. Therefore, the compiler's __builtin_isnan (and friends) cannot
reasonably rely on generating library calls for these.

Probably the best you could say is that we don't support
__builtin_isnan/etc in strict-fp mode. But that's not a good solution,
either.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210902/b1da59cc/attachment.html>