From 0056820a482335551bf7fdbde87eeba9ed220292 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 1 Oct 2014 20:36:33 +0000 Subject: [PATCH] Make the sqrt intrinsic return undef for a negative input. As discussed here: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140609/220598.html And again here: http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/077168.html The sqrt of a negative number when using the llvm intrinsic is undefined. We should return undef rather than 0.0 to match the definition in the LLVM IR lang ref. This change should not affect any code that isn't using "no-nans-fp-math"; ie, no-nans is a requirement for generating the llvm intrinsic in place of a sqrt function call. Unfortunately, the behavior introduced by this patch will not match current gcc, xlc, icc, and possibly other compilers. The current clang/llvm behavior of returning 0.0 doesn't either. We knowingly approve of this difference with the other compilers in an attempt to flag code that is invoking undefined behavior. A front-end warning should also try to convince the user that the program will fail: http://llvm.org/bugs/show_bug.cgi?id=21093 Differential Revision: http://reviews.llvm.org/D5527 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@218803 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ConstantFolding.cpp | 10 ++++++++-- test/Transforms/InstCombine/constant-fold-math.ll | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/Analysis/ConstantFolding.cpp b/lib/Analysis/ConstantFolding.cpp index 88e255832f1..c703491ace2 100644 --- a/lib/Analysis/ConstantFolding.cpp +++ b/lib/Analysis/ConstantFolding.cpp @@ -1520,8 +1520,14 @@ static Constant *ConstantFoldScalarCall(StringRef Name, unsigned IntrinsicID, (Ty->isHalfTy() || Ty->isFloatTy() || Ty->isDoubleTy())) { if (V >= -0.0) return ConstantFoldFP(sqrt, V, Ty); - else // Undefined - return Constant::getNullValue(Ty); + else { + // Unlike the sqrt definitions in C/C++, POSIX, and IEEE-754 - which + // all guarantee or favor returning NaN - the square root of a + // negative number is not defined for the LLVM sqrt intrinsic. + // This is because the intrinsic should only be emitted in place of + // libm's sqrt function when using "no-nans-fp-math". + return UndefValue::get(Ty); + } } break; case 's': diff --git a/test/Transforms/InstCombine/constant-fold-math.ll b/test/Transforms/InstCombine/constant-fold-math.ll index 14377df3729..ce8d337c08b 100644 --- a/test/Transforms/InstCombine/constant-fold-math.ll +++ b/test/Transforms/InstCombine/constant-fold-math.ll @@ -7,6 +7,7 @@ declare <4 x float> @llvm.fma.v4f32(<4 x float>, <4 x float>, <4 x float>) #0 declare double @llvm.fma.f64(double, double, double) #0 declare double @llvm.fmuladd.f64(double, double, double) #0 +declare double @llvm.sqrt.f64(double) #0 ; CHECK-LABEL: @constant_fold_fma_f32 @@ -44,4 +45,12 @@ define double @constant_fold_fmuladd_f64() #0 { ret double %x } +; The sqrt intrinsic is undefined for negative inputs besides -0.0. +; CHECK-LABEL: @bad_sqrt +; CHECK-NEXT: ret double undef +define double @bad_sqrt() { + %x = call double @llvm.sqrt.f64(double -2.000000e+00) + ret double %x +} + attributes #0 = { nounwind readnone } -- 2.34.1