[PATCH] Handle sqrt() shrinking in SimplifyLibCalls like any other call

[Stephen Canon]

(float)sqrt((double)x) —> sqrtf(x) is a valid transformation [See Sam Figueroa’s thesis for details].

However, sqrt((double)x) —> (double)sqrtf(x) is not a valid transformation [Use almost any non-perfect-square x for an example].

– Steve


> On Oct 23, 2014, at 3:35 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> 
> Hrmm... I'm not entirely comfortable with this. Steve, do you have an opinion?
> 
> -Hal
> 
> ----- Original Message -----
>> From: "Sanjay Patel" <spatel at rotateright.com>
>> To: spatel at rotateright.com, beanz at apple.com, hfinkel at anl.gov
>> Cc: llvm-commits at cs.uiuc.edu
>> Sent: Thursday, October 23, 2014 2:24:50 PM
>> Subject: Re: [PATCH] Handle sqrt() shrinking in SimplifyLibCalls like any other call
>> 
>>>> ! In D5919#4, @hfinkel wrote:
>>> Please don't loosen this for the function call (although leave it
>>> as-is for the intrinsic). The intrinsic is different here,
>>> especially because it is defined to have different semantics than
>>> the function call, but we should not change the function call
>>> itself without fast-math enabled. The underlying issue is that
>>> (float) sqrt(x) != sqrtf(x) in general because of rounding issues
>>> (and on some systems, the sqrtf is not exactly 1ulp accurate), and
>>> we should not alter that without fast-math enabled.
>> 
>> Let me make sure we're on the same page. The existing behavior is to
>> transform this function call:
>>   float x;
>>   float y = (float) sqrt ((double) x)
>> 
>> or in IR:
>>   %conv = fpext float %f to double
>>   %call = call double @sqrt(double %conv)
>>   %conv1 = fptrunc double %call to float
>>   ret float %conv1
>> 
>> into:
>>  float y = sqrtf (x);
>> 
>> There's are 2 existing positive test cases in
>> test/Transform/InstCombine/sqrt.ll that check for this pattern and 1
>> negative test case to make sure we don't optimize in the event the
>> result is used as a double.
>> 
>> Are you saying those existing test cases are invalid? I think that
>> the sqrt function call test that I'm proposing to modify in
>> double-float-shrink-1.ll would actually become redundant with the
>> second test case in sqrt.ll; that was added for:
>> http://llvm.org/bugs/show_bug.cgi?id=8096
>> 
>> http://reviews.llvm.org/D5919
>> 
>> 
>> 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory