[llvm-commits] [llvm-gcc-4.2] r82640 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

[Dale Johannesen]

OK, I'll do all this.  May I ask what good llvm.sqrt is when defined  
this way?  It can't be used by any language that follows IEEE754.
And gcc produces NaN for sqrt(-3) even with -ffast-math or -ffinite- 
math-only, so I don't think there is any flag setting where it would  
be appropriate to use llvm.sqrt.

On Sep 23, 2009, at 11:04 PM, Chris Lattner wrote:

> On Sep 23, 2009, at 6:10 PM, Evan Cheng wrote:
> On Sep 23, 2009, at 3:21 PM, Dale Johannesen wrote:
>>> On Sep 23, 2009, at 3:18 PMPDT, Evan Cheng wrote:
>>>
>>>> Are we generating a call to sqrt now? If so, that's bad. We should
>>>> be using SSE sqrts* instructions.
>>>
>>> Agreed.  My proposed semantic change would fix that.
>>
>> You mean change to llvm.sqrt and then llvm-gcc can switch bad to
>> generating the intrinsic? It seems like the current fix is not what  
>> we
>> want. Perhaps we should revert it first?
>>
>> According to Chris, the semantics of sqrt of negative value is  
>> defined
>> and this is just some optimization bug.
>
> Sorry, I'm just catching up on this now.  There are a couple of  
> things that confuse the issue, but I'll just try to keep it "to the  
> point" instead of rambling about history.
>
> 1. I did tell Evan that sqrt is defined on negative number, but I  
> misunderstood and didn't think about llvm.sqrt.  Please disregard my  
> comment Evan.  llvm.sqrt should be undefined on negative numbers as  
> langref says, and llvm-gcc/clang should only transform sqrt to  
> llvm.sqrt if the appropriate "I don't care about fp semantics" flag  
> is set.
>
> 2. Dale's patch to llvm-gcc is ok, but it would be better to still  
> do the transformation when -ffast-math is specified or whatever the  
> more precise "nan's aren't generated" flag is.  We should do the  
> same thing for clang as well.
>
> 3. Please make sure that llvm-gcc/clang on the mac (and other  
> targets with -fno-math-errno) are producing a call to sqrt that is  
> marked as readnone.  Given this, the mid-level optimizer should  
> hoist and cse the calls to sqrt just as well as it did calls to  
> llvm.sqrt.
>
> 4. The constant folding of llvm.sqrt(-123) -> 0 is ok because the  
> intrinsic really is undefined on negative.  The constant folding of  
> sqrt(-123) doesn't fold if the input is negative, so it will just  
> not optimize the curious case.
>
> 5. Please make the X86 backend compile calls to readonly/readnone  
> "sqrt" produce a sqrtsd (etc) instruction.  We really don't want to  
> get a function call on the mac (or other x86 target with -fno-math- 
> errno).  Like malloc, if someone cares about -fno-builtin-sqrt, they  
> can solve the general problem.  We already constant fold "real sqrt"  
> calls in Analysis/ConstantFolding.cpp anyway.
>
> 6. Please add handling of "real sqrt" to llvm::CannotBeNegativeZero  
> to match the handling of llvm.sqrt.
>
> Thanks!
>
> -Chris