From 7cb25a888ce53c288de9369c5b98082b4ea5ec26 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Tue, 31 Oct 2017 20:19:39 +0000 Subject: [PATCH] [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 | 29 +++++++++------------ clang/test/CodeGen/2005-07-20-SqrtNoErrno.c | 10 ------- clang/test/CodeGen/builtin-sqrt.c | 19 ++++++++++++++ clang/test/CodeGen/libcalls.c | 25 +++++++++--------- 4 files changed, 44 insertions(+), 39 deletions(-) delete mode 100644 clang/test/CodeGen/2005-07-20-SqrtNoErrno.c create mode 100644 clang/test/CodeGen/builtin-sqrt.c diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 1de7c6e17d11e..bc37afa28485b 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/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()) - 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()) + return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::sqrt)); + break; case Builtin::BI__builtin_pow: case Builtin::BI__builtin_powf: diff --git a/clang/test/CodeGen/2005-07-20-SqrtNoErrno.c b/clang/test/CodeGen/2005-07-20-SqrtNoErrno.c deleted file mode 100644 index 96761e4cfb2ec..0000000000000 --- a/clang/test/CodeGen/2005-07-20-SqrtNoErrno.c +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s -// llvm.sqrt has undefined behavior on negative inputs, so it is -// inappropriate to translate C/C++ sqrt to this. -float sqrtf(float x); -float foo(float X) { - // CHECK: foo - // CHECK: call float @sqrtf(float % - // Check that this is marked readonly when errno is ignored. - return sqrtf(X); -} diff --git a/clang/test/CodeGen/builtin-sqrt.c b/clang/test/CodeGen/builtin-sqrt.c new file mode 100644 index 0000000000000..f93c5926d5e91 --- /dev/null +++ b/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 {{.*}}} + diff --git a/clang/test/CodeGen/libcalls.c b/clang/test/CodeGen/libcalls.c index 3a8207b2bebc3..0a1bcc605ca88 100644 --- a/clang/test/CodeGen/libcalls.c +++ b/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 }