[PATCH] D27618: Failure to vectorize __builtin_sqrt/__builtin_sqrtf

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:2526
   case LibFunc::sqrtl:
-    if (ICS->hasNoNaNs())
-      return Intrinsic::sqrt;
-    return Intrinsic::not_intrinsic;
+    return Intrinsic::sqrt;
   }
----------------
hfinkel wrote:
> RKSimon wrote:
> > Do we know the history behind why sqrt was protected by this?
> The problem, which has come up a lot, is that our sqrt intrinsic is a special case. Unlike the other intrinsics for math functions, it does not exactly mirror the behavior of the corresponding libm function. As noted in the LangRef, "Unlike sqrt in libm, however, llvm.sqrt has undefined behavior for negative numbers other than -0.0." Thus, unless we get to assume no NaNs, it is possible that the intrinsic might have UB in cases where the libm function does not, and as a result, we can't substitute the intrinsic for the libm call.
I think the checks were added in https://reviews.llvm.org/rL265521 and I think the reasoning behind the check is still sound.

LibFun::sqrt and Intrinsic::sqrt  differ in the way they treat negative elements, the intrinsic has undefined behavior for negative numbers (http://llvm.org/docs/LangRef.html#llvm-sqrt-intrinsic).


https://reviews.llvm.org/D27618