[CodeGen] map sqrt libcalls to llvm.sqrt when errno is not set

The LLVM sqrt intrinsic definition changed with:
D28797
...so we don't have to use any relaxed FP settings other than errno handling.

This patch sidesteps a question raised in PR27435:
https://bugs.llvm.org/show_bug.cgi?id=27435

Is a programmer using __builtin_sqrt() invoking the compiler's intrinsic definition of sqrt or the mathlib definition of sqrt?

But we have an answer now: the builtin should match the behavior of the libm function including errno handling.

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

llvm-svn: 317031

clang/lib/CodeGen/CGBuiltin.cpp

@@ -2072,24 +2072,21 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
     return RValue::get(nullptr);
   }
 
-    // Library functions with special handling.
   case Builtin::BIsqrt:
   case Builtin::BIsqrtf:
-  case Builtin::BIsqrtl: {
-    // Transform a call to sqrt* into a @llvm.sqrt.* intrinsic call, but only
-    // in finite- or unsafe-math mode (the intrinsic has different semantics
-    // for handling negative numbers compared to the library function, so
-    // -fmath-errno=0 is not enough).
-    if (!FD->hasAttr<ConstAttr>())
-      break;
-    if (!(CGM.getCodeGenOpts().UnsafeFPMath ||
-          CGM.getCodeGenOpts().NoNaNsFPMath))
-      break;
-    Value *Arg0 = EmitScalarExpr(E->getArg(0));
-    llvm::Type *ArgType = Arg0->getType();
-    Value *F = CGM.getIntrinsic(Intrinsic::sqrt, ArgType);
-    return RValue::get(Builder.CreateCall(F, Arg0));
-  }
+  case Builtin::BIsqrtl:
+    // Builtins have the same semantics as library functions. The LLVM intrinsic
+    // has the same semantics as the library function except it does not set
+    // errno. Thus, we can transform either sqrt or __builtin_sqrt to @llvm.sqrt
+    // if the call is 'const' (the call must not set errno).
+    //
+    // FIXME: The builtin cases are not here because they are marked 'const' in
+    // Builtins.def. So that means they are wrongly defined to have different
+    // semantics than the library functions. If we included them here, we would
+    // turn them into LLVM intrinsics regardless of whether -fmath-errno was on.
+    if (FD->hasAttr<ConstAttr>())
+      return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::sqrt));
+    break;
 
   case Builtin::BI__builtin_pow:
   case Builtin::BI__builtin_powf:

clang/test/CodeGen/builtin-sqrt.c

@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fmath-errno -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s --check-prefix=HAS_ERRNO
+// RUN: %clang_cc1              -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s --check-prefix=NO_ERRNO
+
+// FIXME: If a builtin is supposed to have identical semantics to its libm twin, then it
+// should not be marked "constant" in Builtins.def because that means it can't set errno.
+// Note that both runs have 'readnone' on the libcall here.
+
+float foo(float X) {
+  // HAS_ERRNO: call float @sqrtf(float
+  // NO_ERRNO: call float @sqrtf(float
+  return __builtin_sqrtf(X);
+}
+
+// HAS_ERRNO: declare float @sqrtf(float) [[ATTR:#[0-9]+]]
+// HAS_ERRNO: attributes [[ATTR]] = { nounwind readnone {{.*}}}
+
+// NO_ERRNO: declare float @sqrtf(float) [[ATTR:#[0-9]+]]
+// NO_ERRNO: attributes [[ATTR]] = { nounwind readnone {{.*}}}
+

clang/test/CodeGen/libcalls.c

@@ -6,29 +6,28 @@
 // CHECK-NO-LABEL: define void @test_sqrt
 // CHECK-FAST-LABEL: define void @test_sqrt
 void test_sqrt(float a0, double a1, long double a2) {
-  // Following llvm-gcc's lead, we never emit these as intrinsics;
-  // no-math-errno isn't good enough.  We could probably use intrinsics
-  // with appropriate guards if it proves worthwhile.
-
   // CHECK-YES: call float @sqrtf
-  // CHECK-NO: call float @sqrtf
+  // CHECK-NO: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
-  // CHECK-NO: call double @sqrt
+  // CHECK-NO: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);
 
   // CHECK-YES: call x86_fp80 @sqrtl
-  // CHECK-NO: call x86_fp80 @sqrtl
+  // CHECK-NO: call x86_fp80 @llvm.sqrt.f80(x86_fp80
+  // CHECK-FAST: call x86_fp80 @llvm.sqrt.f80(x86_fp80
   long double l2 = sqrtl(a2);
 }
 
 // CHECK-YES: declare float @sqrtf(float)
 // CHECK-YES: declare double @sqrt(double)
 // CHECK-YES: declare x86_fp80 @sqrtl(x86_fp80)
-// CHECK-NO: declare float @sqrtf(float) [[NUW_RN:#[0-9]+]]
-// CHECK-NO: declare double @sqrt(double) [[NUW_RN]]
-// CHECK-NO: declare x86_fp80 @sqrtl(x86_fp80) [[NUW_RN]]
+// CHECK-NO: declare float @llvm.sqrt.f32(float)
+// CHECK-NO: declare double @llvm.sqrt.f64(double)
+// CHECK-NO: declare x86_fp80 @llvm.sqrt.f80(x86_fp80)
 // CHECK-FAST: declare float @llvm.sqrt.f32(float)
 // CHECK-FAST: declare double @llvm.sqrt.f64(double)
 // CHECK-FAST: declare x86_fp80 @llvm.sqrt.f80(x86_fp80)
@@ -86,7 +85,7 @@ void test_builtins(double d, float f, long double ld) {
   double atan_ = atan(d);
   long double atanl_ = atanl(ld);
   float atanf_ = atanf(f);
-// CHECK-NO: declare double @atan(double) [[NUW_RN]]
+// CHECK-NO: declare double @atan(double) [[NUW_RN:#[0-9]+]]
 // CHECK-NO: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atanf(float) [[NUW_RN]]
 // CHECK-YES-NOT: declare double @atan(double) [[NUW_RN]]
@@ -126,5 +125,5 @@ void test_builtins(double d, float f, long double ld) {
 
 // CHECK-YES: attributes [[NUW_RN]] = { nounwind readnone speculatable }
 
-// CHECK-NO: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
-// CHECK-NO: attributes [[NUW_RNI]] = { nounwind readnone speculatable }
+// CHECK-NO-DAG: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
+// CHECK-NO-DAG: attributes [[NUW_RNI]] = { nounwind readnone speculatable }