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

[Hal Finkel]

----- Original Message -----
> From: "Stephen Canon" <scanon at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu, spatel at rotateright.com, beanz at apple.com,
> reviews+D5919+public+763adead774985f1 at reviews.llvm.org
> Sent: Thursday, October 23, 2014 2:46:17 PM
> Subject: Re: [PATCH] Handle sqrt() shrinking in SimplifyLibCalls like any other call
> 
> (float)sqrt((double)x) —> sqrtf(x) is a valid transformation [See Sam
> Figueroa’s thesis for details].

Does this assume that the sqrtf is correct to <= 0.5ulps?

 -Hal

> 
> 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
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory