[llvm] 32c9991 - [InstCombine] Fix errno bug in pow expansion to sqrt
Hubert Tong via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 22 15:59:59 PDT 2020
Author: Hubert Tong
Date: 2020-09-22T18:58:05-04:00
New Revision: 32c9991dab5cb1454959561c77f9d0089d981429
URL: https://github.com/llvm/llvm-project/commit/32c9991dab5cb1454959561c77f9d0089d981429
DIFF: https://github.com/llvm/llvm-project/commit/32c9991dab5cb1454959561c77f9d0089d981429.diff
LOG: [InstCombine] Fix errno bug in pow expansion to sqrt
A conversion from `pow` to `sqrt` shall not call an `errno`-setting
`sqrt` with -//infinity//: the `sqrt` will set `EDOM` where the `pow`
call need not.
This patch avoids the erroneous (pun not intended) transformation by
applying the restrictions discussed in the thread for
https://lists.llvm.org/pipermail/llvm-dev/2020-September/145051.html.
The existing tests are updated (depending on emphasis in the checks for
library calls, avoidance of overlap, and overall coverage):
- to add `ninf`, retaining the intended library call,
- to use the intrinsic, retaining the use of `select`, or
- to expect the replacement to not occur.
The following is tested:
- The pow intrinsic folds to a `select` instruction to
handle -//infinity//.
- The pow library call folds, with `ninf`, to `sqrt` without the
`select` instruction associated with handling -//infinity//.
- The pow library call does not fold to `sqrt` without `ninf`.
Reviewed By: spatel
Differential Revision: https://reviews.llvm.org/D87877
Added:
Modified:
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
llvm/test/Transforms/InstCombine/pow-1.ll
llvm/test/Transforms/InstCombine/pow-sqrt.ll
llvm/test/Transforms/InstCombine/win-math.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index c4eff0795d12..bcda3f3440a3 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -1636,6 +1636,14 @@ Value *LibCallSimplifier::replacePowWithSqrt(CallInst *Pow, IRBuilderBase &B) {
if (ExpoF->isNegative() && (!Pow->hasApproxFunc() && !Pow->hasAllowReassoc()))
return nullptr;
+ // If we have a pow() library call (accesses memory) and we can't guarantee
+ // that the base is not an infinity, give up:
+ // pow(-Inf, 0.5) is optionally required to have a result of +Inf (not setting
+ // errno), but sqrt(-Inf) is required by various standards to set errno.
+ if (!Pow->doesNotAccessMemory() && !Pow->hasNoInfs() &&
+ !isKnownNeverInfinity(Base, TLI))
+ return nullptr;
+
Sqrt = getSqrtCall(Base, Attrs, Pow->doesNotAccessMemory(), Mod, B, TLI);
if (!Sqrt)
return nullptr;
diff --git a/llvm/test/Transforms/InstCombine/pow-1.ll b/llvm/test/Transforms/InstCombine/pow-1.ll
index dfb62f6d0af0..22da98b96eec 100644
--- a/llvm/test/Transforms/InstCombine/pow-1.ll
+++ b/llvm/test/Transforms/InstCombine/pow-1.ll
@@ -19,6 +19,7 @@
; in the cases below where pow is transformed into another function call.
declare float @powf(float, float) nounwind readonly
+declare float @llvm.pow.f32(float, float)
declare double @pow(double, double) nounwind readonly
declare double @llvm.pow.f64(double, double)
declare <2 x float> @llvm.pow.v2f32(<2 x float>, <2 x float>) nounwind readonly
@@ -247,43 +248,34 @@ define <2 x double> @test_simplify6v(<2 x double> %x) {
; Check pow(x, 0.5) -> fabs(sqrt(x)), where x != -infinity.
-define float @powf_libcall_to_select_sqrt(float %x) {
-; CHECK-LABEL: @powf_libcall_to_select_sqrt(
-; ANY-NEXT: [[SQRTF:%.*]] = call float @sqrtf(float [[X:%.*]])
-; ANY-NEXT: [[ABS:%.*]] = call float @llvm.fabs.f32(float [[SQRTF]])
-; ANY-NEXT: [[ISINF:%.*]] = fcmp oeq float [[X]], 0xFFF0000000000000
-; ANY-NEXT: [[TMP1:%.*]] = select i1 [[ISINF]], float 0x7FF0000000000000, float [[ABS]]
-; ANY-NEXT: ret float [[TMP1]]
-; VC32-NEXT: [[POW:%.*]] = call float @powf(float [[X:%.*]], float 5.000000e-01)
+define float @powf_libcall_half_ninf(float %x) {
+; CHECK-LABEL: @powf_libcall_half_ninf(
+; ANY-NEXT: [[SQRTF:%.*]] = call ninf float @sqrtf(float [[X:%.*]])
+; ANY-NEXT: [[ABS:%.*]] = call ninf float @llvm.fabs.f32(float [[SQRTF]])
+; ANY-NEXT: ret float [[ABS]]
+; VC32-NEXT: [[POW:%.*]] = call ninf float @powf(float [[X:%.*]], float 5.000000e-01)
; VC32-NEXT: ret float [[POW]]
-; VC51-NEXT: [[POW:%.*]] = call float @powf(float [[X:%.*]], float 5.000000e-01)
+; VC51-NEXT: [[POW:%.*]] = call ninf float @powf(float [[X:%.*]], float 5.000000e-01)
; VC51-NEXT: ret float [[POW]]
-; VC64-NEXT: [[SQRTF:%.*]] = call float @sqrtf(float [[X:%.*]])
-; VC64-NEXT: [[ABS:%.*]] = call float @llvm.fabs.f32(float [[SQRTF]])
-; VC64-NEXT: [[ISINF:%.*]] = fcmp oeq float [[X]], 0xFFF0000000000000
-; VC64-NEXT: [[TMP1:%.*]] = select i1 [[ISINF]], float 0x7FF0000000000000, float [[ABS]]
-; VC64-NEXT: ret float [[TMP1]]
-; VC83-NEXT: [[SQRTF:%.*]] = call float @sqrtf(float [[X:%.*]])
-; VC83-NEXT: [[ABS:%.*]] = call float @llvm.fabs.f32(float [[SQRTF]])
-; VC83-NEXT: [[ISINF:%.*]] = fcmp oeq float [[X]], 0xFFF0000000000000
-; VC83-NEXT: [[TMP1:%.*]] = select i1 [[ISINF]], float 0x7FF0000000000000, float [[ABS]]
-; VC83-NEXT: ret float [[TMP1]]
-; NOLIB-NEXT: [[POW:%.*]] = call float @powf(float [[X:%.*]], float 5.000000e-01)
+; VC64-NEXT: [[SQRTF:%.*]] = call ninf float @sqrtf(float [[X:%.*]])
+; VC64-NEXT: [[ABS:%.*]] = call ninf float @llvm.fabs.f32(float [[SQRTF]])
+; VC64-NEXT: ret float [[ABS]]
+; VC83-NEXT: [[SQRTF:%.*]] = call ninf float @sqrtf(float [[X:%.*]])
+; VC83-NEXT: [[ABS:%.*]] = call ninf float @llvm.fabs.f32(float [[SQRTF]])
+; VC83-NEXT: ret float [[ABS]]
+; NOLIB-NEXT: [[POW:%.*]] = call ninf float @powf(float [[X:%.*]], float 5.000000e-01)
; NOLIB-NEXT: ret float [[POW]]
;
- %retval = call float @powf(float %x, float 0.5)
+ %retval = call ninf float @powf(float %x, float 0.5)
ret float %retval
}
-define double @pow_libcall_to_select_sqrt(double %x) {
-; CHECK-LABEL: @pow_libcall_to_select_sqrt(
-; LIB-NEXT: [[SQRT:%.*]] = call double @sqrt(double [[X:%.*]])
-; LIB-NEXT: [[ABS:%.*]] = call double @llvm.fabs.f64(double [[SQRT]])
-; LIB-NEXT: [[ISINF:%.*]] = fcmp oeq double [[X]], 0xFFF0000000000000
-; LIB-NEXT: [[TMP1:%.*]] = select i1 [[ISINF]], double 0x7FF0000000000000, double [[ABS]]
-; LIB-NEXT: ret double [[TMP1]]
-; NOLIB-NEXT: [[POW:%.*]] = call double @pow(double [[X:%.*]], double 5.000000e-01)
-; NOLIB-NEXT: ret double [[POW]]
+; Check pow(x, 0.5) where x may be -infinity does not call a library sqrt function.
+
+define double @pow_libcall_half_no_FMF(double %x) {
+; CHECK-LABEL: @pow_libcall_half_no_FMF(
+; CHECK-NEXT: [[POW:%.*]] = call double @pow(double [[X:%.*]], double 5.000000e-01)
+; CHECK-NEXT: ret double [[POW]]
;
%retval = call double @pow(double %x, double 0.5)
ret double %retval
@@ -293,27 +285,17 @@ define double @pow_libcall_to_select_sqrt(double %x) {
define float @test_simplify9(float %x) {
; CHECK-LABEL: @test_simplify9(
-; ANY-NEXT: ret float 0x7FF0000000000000
-; VC32-NEXT: [[POW:%.*]] = call float @powf(float 0xFFF0000000000000, float 5.000000e-01)
-; VC32-NEXT: ret float [[POW]]
-; VC51-NEXT: [[POW:%.*]] = call float @powf(float 0xFFF0000000000000, float 5.000000e-01)
-; VC51-NEXT: ret float [[POW]]
-; VC64-NEXT: ret float 0x7FF0000000000000
-; VC83-NEXT: ret float 0x7FF0000000000000
-; NOLIB-NEXT: [[POW:%.*]] = call float @powf(float 0xFFF0000000000000, float 5.000000e-01)
-; NOLIB-NEXT: ret float [[POW]]
+; CHECK-NEXT: ret float 0x7FF0000000000000
;
- %retval = call float @powf(float 0xFFF0000000000000, float 0.5)
+ %retval = call float @llvm.pow.f32(float 0xFFF0000000000000, float 0.5)
ret float %retval
}
define double @test_simplify10(double %x) {
; CHECK-LABEL: @test_simplify10(
-; LIB-NEXT: ret double 0x7FF0000000000000
-; NOLIB-NEXT: [[POW:%.*]] = call double @pow(double 0xFFF0000000000000, double 5.000000e-01)
-; NOLIB-NEXT: ret double [[POW]]
+; CHECK-NEXT: ret double 0x7FF0000000000000
;
- %retval = call double @pow(double 0xFFF0000000000000, double 0.5)
+ %retval = call double @llvm.pow.f64(double 0xFFF0000000000000, double 0.5)
ret double %retval
}
@@ -482,8 +464,8 @@ define <2 x double> @pow_neg1_double_fastv(<2 x double> %x) {
ret <2 x double> %r
}
-define double @test_simplify17(double %x) {
-; CHECK-LABEL: @test_simplify17(
+define double @pow_intrinsic_half_no_FMF(double %x) {
+; CHECK-LABEL: @pow_intrinsic_half_no_FMF(
; CHECK-NEXT: [[SQRT:%.*]] = call double @llvm.sqrt.f64(double [[X:%.*]])
; CHECK-NEXT: [[ABS:%.*]] = call double @llvm.fabs.f64(double [[SQRT]])
; CHECK-NEXT: [[ISINF:%.*]] = fcmp oeq double [[X]], 0xFFF0000000000000
diff --git a/llvm/test/Transforms/InstCombine/pow-sqrt.ll b/llvm/test/Transforms/InstCombine/pow-sqrt.ll
index 588d4d1293db..38f64d5584ff 100644
--- a/llvm/test/Transforms/InstCombine/pow-sqrt.ll
+++ b/llvm/test/Transforms/InstCombine/pow-sqrt.ll
@@ -3,20 +3,19 @@
; Check the libcall and the intrinsic for each case with
diff ering FMF.
-; The transform to sqrt is allowed as long as we deal with -0.0 and -INF.
+; The transform to sqrt is not allowed if we risk setting errno due to -INF.
define double @pow_libcall_half_no_FMF(double %x) {
; CHECK-LABEL: @pow_libcall_half_no_FMF(
-; CHECK-NEXT: [[SQRT:%.*]] = call double @sqrt(double [[X:%.*]])
-; CHECK-NEXT: [[ABS:%.*]] = call double @llvm.fabs.f64(double [[SQRT]])
-; CHECK-NEXT: [[ISINF:%.*]] = fcmp oeq double [[X]], 0xFFF0000000000000
-; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISINF]], double 0x7FF0000000000000, double [[ABS]]
-; CHECK-NEXT: ret double [[TMP1]]
+; CHECK-NEXT: [[POW:%.*]] = call double @pow(double [[X:%.*]], double 5.000000e-01)
+; CHECK-NEXT: ret double [[POW]]
;
%pow = call double @pow(double %x, double 5.0e-01)
ret double %pow
}
+; The transform to (non-errno setting) sqrt is allowed as long as we deal with -0.0 and -INF.
+
define double @pow_intrinsic_half_no_FMF(double %x) {
; CHECK-LABEL: @pow_intrinsic_half_no_FMF(
; CHECK-NEXT: [[SQRT:%.*]] = call double @llvm.sqrt.f64(double [[X:%.*]])
@@ -29,20 +28,25 @@ define double @pow_intrinsic_half_no_FMF(double %x) {
ret double %pow
}
-; This makes no
diff erence, but FMF are propagated.
+; `afn` makes no
diff erence, but FMF are propagated/retained.
+
+; (As above) the transform to sqrt may generate EDOM due to -INF. Generally, EDOM implies
+; formation of a NaN (which then propagates). `afn` may justify returning NaN (along with
+; setting EDOM); however, the conservatively correct approach is to avoid both the NaN and
+; the EDOM.
define double @pow_libcall_half_approx(double %x) {
; CHECK-LABEL: @pow_libcall_half_approx(
-; CHECK-NEXT: [[SQRT:%.*]] = call afn double @sqrt(double [[X:%.*]])
-; CHECK-NEXT: [[ABS:%.*]] = call afn double @llvm.fabs.f64(double [[SQRT]])
-; CHECK-NEXT: [[ISINF:%.*]] = fcmp afn oeq double [[X]], 0xFFF0000000000000
-; CHECK-NEXT: [[TMP1:%.*]] = select afn i1 [[ISINF]], double 0x7FF0000000000000, double [[ABS]]
-; CHECK-NEXT: ret double [[TMP1]]
+; CHECK-NEXT: [[POW:%.*]] = call afn double @pow(double [[X:%.*]], double 5.000000e-01)
+; CHECK-NEXT: ret double [[POW]]
;
%pow = call afn double @pow(double %x, double 5.0e-01)
ret double %pow
}
+; (As above) the transform to (non-errno setting) sqrt is allowed as long as we deal with -0.0
+; and -INF.
+
define <2 x double> @pow_intrinsic_half_approx(<2 x double> %x) {
; CHECK-LABEL: @pow_intrinsic_half_approx(
; CHECK-NEXT: [[SQRT:%.*]] = call afn <2 x double> @llvm.sqrt.v2f64(<2 x double> [[X:%.*]])
@@ -86,14 +90,12 @@ define <2 x double> @pow_intrinsic_half_ninf(<2 x double> %x) {
ret <2 x double> %pow
}
-; If we can disregard -0.0, no need for fabs.
+; If we can disregard -0.0, no need for fabs, but still (because of -INF) cannot use library sqrt.
define double @pow_libcall_half_nsz(double %x) {
; CHECK-LABEL: @pow_libcall_half_nsz(
-; CHECK-NEXT: [[SQRT:%.*]] = call nsz double @sqrt(double [[X:%.*]])
-; CHECK-NEXT: [[ISINF:%.*]] = fcmp nsz oeq double [[X]], 0xFFF0000000000000
-; CHECK-NEXT: [[TMP1:%.*]] = select nsz i1 [[ISINF]], double 0x7FF0000000000000, double [[SQRT]]
-; CHECK-NEXT: ret double [[TMP1]]
+; CHECK-NEXT: [[POW:%.*]] = call nsz double @pow(double [[X:%.*]], double 5.000000e-01)
+; CHECK-NEXT: ret double [[POW]]
;
%pow = call nsz double @pow(double %x, double 5.0e-01)
ret double %pow
@@ -162,35 +164,27 @@ define float @pow_libcall_neghalf_no_FMF(float %x) {
ret float %pow
}
+; If we can disregard INFs, a call to a library sqrt is okay.
; Transform to sqrt+fdiv because 'reassoc' allows an extra rounding step.
; Use 'fabs' to handle -0.0 correctly.
-; Use 'select' to handle -INF correctly.
-define float @pow_libcall_neghalf_reassoc(float %x) {
-; CHECK-LABEL: @pow_libcall_neghalf_reassoc(
-; CHECK-NEXT: [[SQRTF:%.*]] = call reassoc float @sqrtf(float [[X:%.*]])
-; CHECK-NEXT: [[ABS:%.*]] = call reassoc float @llvm.fabs.f32(float [[SQRTF]])
-; CHECK-NEXT: [[ISINF:%.*]] = fcmp reassoc oeq float [[X]], 0xFFF0000000000000
-; CHECK-NEXT: [[ABS_OP:%.*]] = fdiv reassoc float 1.000000e+00, [[ABS]]
-; CHECK-NEXT: [[RECIPROCAL:%.*]] = select i1 [[ISINF]], float 0.000000e+00, float [[ABS_OP]]
+define float @pow_libcall_neghalf_reassoc_ninf(float %x) {
+; CHECK-LABEL: @pow_libcall_neghalf_reassoc_ninf(
+; CHECK-NEXT: [[SQRTF:%.*]] = call reassoc ninf float @sqrtf(float [[X:%.*]])
+; CHECK-NEXT: [[ABS:%.*]] = call reassoc ninf float @llvm.fabs.f32(float [[SQRTF]])
+; CHECK-NEXT: [[RECIPROCAL:%.*]] = fdiv reassoc ninf float 1.000000e+00, [[ABS]]
; CHECK-NEXT: ret float [[RECIPROCAL]]
;
- %pow = call reassoc float @powf(float %x, float -5.0e-01)
+ %pow = call reassoc ninf float @powf(float %x, float -5.0e-01)
ret float %pow
}
-; Transform to sqrt+fdiv because 'afn' allows an extra rounding step.
-; Use 'fabs' to handle -0.0 correctly.
-; Use 'select' to handle -INF correctly.
+; If we cannot disregard INFs, a call to a library sqrt is not okay.
define float @pow_libcall_neghalf_afn(float %x) {
; CHECK-LABEL: @pow_libcall_neghalf_afn(
-; CHECK-NEXT: [[SQRTF:%.*]] = call afn float @sqrtf(float [[X:%.*]])
-; CHECK-NEXT: [[ABS:%.*]] = call afn float @llvm.fabs.f32(float [[SQRTF]])
-; CHECK-NEXT: [[ISINF:%.*]] = fcmp afn oeq float [[X]], 0xFFF0000000000000
-; CHECK-NEXT: [[ABS_OP:%.*]] = fdiv afn float 1.000000e+00, [[ABS]]
-; CHECK-NEXT: [[RECIPROCAL:%.*]] = select i1 [[ISINF]], float 0.000000e+00, float [[ABS_OP]]
-; CHECK-NEXT: ret float [[RECIPROCAL]]
+; CHECK-NEXT: [[POW:%.*]] = call afn float @powf(float [[X:%.*]], float -5.000000e-01)
+; CHECK-NEXT: ret float [[POW]]
;
%pow = call afn float @powf(float %x, float -5.0e-01)
ret float %pow
@@ -265,15 +259,12 @@ define <2 x double> @pow_intrinsic_neghalf_ninf(<2 x double> %x) {
ret <2 x double> %pow
}
-; If we can disregard -0.0, no need for fabs.
+; If we can disregard -0.0, no need for fabs, but still (because of -INF) cannot use library sqrt.
define double @pow_libcall_neghalf_nsz(double %x) {
; CHECK-LABEL: @pow_libcall_neghalf_nsz(
-; CHECK-NEXT: [[SQRT:%.*]] = call nsz afn double @sqrt(double [[X:%.*]])
-; CHECK-NEXT: [[ISINF:%.*]] = fcmp nsz afn oeq double [[X]], 0xFFF0000000000000
-; CHECK-NEXT: [[SQRT_OP:%.*]] = fdiv nsz afn double 1.000000e+00, [[SQRT]]
-; CHECK-NEXT: [[RECIPROCAL:%.*]] = select i1 [[ISINF]], double 0.000000e+00, double [[SQRT_OP]]
-; CHECK-NEXT: ret double [[RECIPROCAL]]
+; CHECK-NEXT: [[POW:%.*]] = call nsz afn double @pow(double [[X:%.*]], double -5.000000e-01)
+; CHECK-NEXT: ret double [[POW]]
;
%pow = call afn nsz double @pow(double %x, double -5.0e-01)
ret double %pow
diff --git a/llvm/test/Transforms/InstCombine/win-math.ll b/llvm/test/Transforms/InstCombine/win-math.ll
index 38ed949e949d..54b3d7b2666a 100644
--- a/llvm/test/Transforms/InstCombine/win-math.ll
+++ b/llvm/test/Transforms/InstCombine/win-math.ll
@@ -330,6 +330,6 @@ define float @float_powsqrt(float %x) nounwind readnone {
; MINGW64-NOT: float @powf
; MINGW64: float @sqrtf
; MINGW64: float @llvm.fabs.f32(
- %1 = call float @powf(float %x, float 0.5)
+ %1 = call ninf float @powf(float %x, float 0.5)
ret float %1
}
More information about the llvm-commits
mailing list