[PATCH] D28797: [LangRef] Make @llvm.sqrt(x) return undef, rather than have UB, for negative x.

[Justin Lebar via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293242: [LangRef] Make @llvm.sqrt(x) return undef, rather than have UB, for negative x. (authored by jlebar).

Changed prior to commit:
  https://reviews.llvm.org/D28797?vs=84744&id=85990#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28797

Files:
  llvm/trunk/docs/LangRef.rst
  llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp


Index: llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
===================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -1097,8 +1097,11 @@
       IRBuilder<>::FastMathFlagGuard Guard(B);
       B.setFastMathFlags(CI->getFastMathFlags());
 
-      // Here we cannot lower to an intrinsic because C99 sqrt() and llvm.sqrt
-      // are not guaranteed to have the same semantics.
+      // TODO: If the pow call is an intrinsic, we should lower to the sqrt
+      // intrinsic, so we match errno semantics.  We also should check that the
+      // target can in fact lower the sqrt intrinsic -- we currently have no way
+      // to ask this question other than asking whether the target has a sqrt
+      // libcall, which is a sufficient but not necessary condition.
       Value *Sqrt = emitUnaryFloatFnCall(Op1, TLI->getName(LibFunc_sqrt), B,
                                          Callee->getAttributes());
 
@@ -1115,8 +1118,8 @@
       IRBuilder<>::FastMathFlagGuard Guard(B);
       B.setFastMathFlags(CI->getFastMathFlags());
 
-      // Unlike other math intrinsics, sqrt has differerent semantics
-      // from the libc function. See LangRef for details.
+      // TODO: As above, we should lower to the sqrt intrinsic if the pow is an
+      // intrinsic, to match errno semantics.
       return emitUnaryFloatFnCall(Op1, TLI->getName(LibFunc_sqrt), B,
                                   Callee->getAttributes());
     }
@@ -1127,6 +1130,9 @@
     // TODO: In finite-only mode, this could be just fabs(sqrt(x)).
     Value *Inf = ConstantFP::getInfinity(CI->getType());
     Value *NegInf = ConstantFP::getInfinity(CI->getType(), true);
+
+    // TODO: As above, we should lower to the sqrt intrinsic if the pow is an
+    // intrinsic, to match errno semantics.
     Value *Sqrt = emitUnaryFloatFnCall(Op1, "sqrt", B, Callee->getAttributes());
 
     Module *M = Callee->getParent();
@@ -1309,8 +1315,11 @@
 Value *LibCallSimplifier::optimizeSqrt(CallInst *CI, IRBuilder<> &B) {
   Function *Callee = CI->getCalledFunction();
   Value *Ret = nullptr;
+  // TODO: Once we have a way (other than checking for the existince of the
+  // libcall) to tell whether our target can lower @llvm.sqrt, relax the
+  // condition below.
   if (TLI->has(LibFunc_sqrtf) && (Callee->getName() == "sqrt" ||
-                             Callee->getIntrinsicID() == Intrinsic::sqrt))
+                                  Callee->getIntrinsicID() == Intrinsic::sqrt))
     Ret = optimizeUnaryDoubleFP(CI, B, true);
 
   if (!CI->hasUnsafeAlgebra())
Index: llvm/trunk/docs/LangRef.rst
===================================================================
--- llvm/trunk/docs/LangRef.rst
+++ llvm/trunk/docs/LangRef.rst
@@ -10076,11 +10076,8 @@
 """""""""
 
 The '``llvm.sqrt``' intrinsics return the sqrt of the specified operand,
-returning the same value as the libm '``sqrt``' functions would. Unlike
-``sqrt`` in libm, however, ``llvm.sqrt`` has undefined behavior for
-negative numbers other than -0.0 (which allows for better optimization,
-because there is no need to worry about errno being set).
-``llvm.sqrt(-0.0)`` is defined to return -0.0 like IEEE sqrt.
+returning the same value as the libm '``sqrt``' functions would, but without
+trapping or setting ``errno``.
 
 Arguments:
 """"""""""


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28797.85990.patch
Type: text/x-patch
Size: 3421 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170127/0f0bd230/attachment.bin>