Skip to content

Commit

Permalink
[LangRef] Make @llvm.sqrt(x) return undef, rather than have UB, for n…
Browse files Browse the repository at this point in the history
…egative x.

Summary:
Some frontends emit a speculate-and-select idiom for sqrt, wherein they compute
sqrt(x), check if x is negative, and select NaN if it is:

  %cmp = fcmp olt double %a, -0.000000e+00
  %sqrt = call double @llvm.sqrt.f64(double %a)
  %ret = select i1 %cmp, double 0x7FF8000000000000, double %sqrt

This is technically UB as the LangRef is written today if %a is ever less than
-0.  But emitting code that's compliant with the current definition of sqrt
would require a branch, which would then prevent us from matching this idiom in
SelectionDAG (which we do today -- ISD::FSQRT has defined behavior on negative
inputs), because SelectionDAG looks at one BB at a time.

Nothing in LLVM takes advantage of this undefined behavior, as far as we can
tell, and the fact that llvm.sqrt has UB dates from its initial addition to the
LangRef.

Reviewers: arsenm, mehdi_amini, hfinkel

Subscribers: wdng, llvm-commits

Differential Revision: https://reviews.llvm.org/D28797

llvm-svn: 293242
  • Loading branch information
Justin Lebar committed Jan 27, 2017
1 parent a95ff38 commit cb9b41d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 10 deletions.
7 changes: 2 additions & 5 deletions llvm/docs/LangRef.rst
Expand Up @@ -10076,11 +10076,8 @@ Overview:
"""""""""

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:
""""""""""
Expand Down
19 changes: 14 additions & 5 deletions llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
Expand Up @@ -1097,8 +1097,11 @@ Value *LibCallSimplifier::optimizePow(CallInst *CI, IRBuilder<> &B) {
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());

Expand All @@ -1115,8 +1118,8 @@ Value *LibCallSimplifier::optimizePow(CallInst *CI, IRBuilder<> &B) {
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());
}
Expand All @@ -1127,6 +1130,9 @@ Value *LibCallSimplifier::optimizePow(CallInst *CI, IRBuilder<> &B) {
// 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();
Expand Down Expand Up @@ -1309,8 +1315,11 @@ Value *LibCallSimplifier::optimizeLog(CallInst *CI, IRBuilder<> &B) {
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())
Expand Down

0 comments on commit cb9b41d

Please sign in to comment.