[llvm-dev] Invalid transformation in LibCallSimplifier::replacePowWithSqrt?

[Sanjay Patel via llvm-dev]

Yes, I mean just bail out on the transform in
LibCallSimplifier::replacePowWithSqrt() -> getSqrtCall(). If we can't prove
the call behaves the same with errno, then give up.
I'm not sure where the select / branching happens, but I don't see that
happening in the initial transform (called from -instcombine)

On Mon, Sep 14, 2020 at 12:58 PM Hubert Tong <
hubert.reinterpretcast at gmail.com> wrote:

> On Mon, Sep 14, 2020 at 12:45 PM Sanjay Patel <spatel at rotateright.com>
> wrote:
>
>> Yes, that looks like a bug. The transform is ok in general for negative
>> numbers, but -Inf is a special-case for pow(), right?
>> If so, we probably need an extra check of the input with
>> "isKnownNeverInfinity()".
>>
> There is an extra check there already, but it uses "select" instead of
> branching. One question is if branching is okay to use instead. Or perhaps
> you mean the transform should not be done unless "isKnownNeverInfinity()"
> returns true.
>
>
>> If there are other errno divergences for edge case values, we may need to
>> check other conditions.
>>
>> On Sat, Sep 12, 2020 at 9:37 PM Hubert Tong via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> The transformation in LibCallSimplifier::replacePowWithSqrt with
>>> respect to -Inf uses a select instruction, which based on the observed
>>> behaviour, incorporates the side effects of the unchosen branch. This means
>>> that (for pow) a call to sqrt(-Inf) is materialized. Such a call is
>>> specified as having a domain error (C17 subclause 7.12.7.5) since the
>>> operand is less than zero. Contrast this with pow(-Inf, 0.5), which is
>>> specified by C17 subclause F.10.4.4 as having a result of +Inf (indicating
>>> an exact result for the operation and, since IEEE Std 754-2008 subclause
>>> 9.1.1 states that domain errors are to be indicated by a NaN result, a lack
>>> of a domain error).
>>>
>>> It is noted that the above statements were made notwithstanding the
>>> ERRORS section of pow() in POSIX.1-2017 XSH Chapter 3, which specifies a
>>> domain error except perhaps by deference to the C standard due to a
>>> conflict between the POSIX and the C wording.
>>>
>>> The transformation in question causes EDOM for  pow(-Inf, 0.5) even on
>>> platforms where the system library implementation of pow does not cause
>>> this situation to arise.
>>>
>>> A sample program that (on some platforms, such as Linux on x86-64)
>>> completes successfully with optimizations off, and aborts with LLVM's
>>> optimization follows; -fmath-errno does not help, and it is not
>>> expected to, because it is designed to retain setting errno to non-zero
>>> (not to prevent spuriously setting errno).
>>>
>>> #include <errno.h>
>>> volatile double inf = -__builtin_inf();
>>>
>>> double pow(double, double);
>>> void abort(void);
>>> int main(void) {
>>>   errno = 0;
>>>   pow(inf, 0.5);
>>>   if (errno != 0) abort();
>>> }
>>>
>>> Compiler Explorer link: https://godbolt.org/z/5Wr66M
>>>
>>> Is the transformation actually valid in some way? If not, I see no
>>> instances where the LibCallSimplier generates conditional branches.
>>> Retaining the transformation in some way without generating conditional
>>> branches would probably involve bailing out if infinities are still in play.
>>>
>>> _______________________________________________
>>> 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/20200914/8facfb22/attachment.html>