This is an archive of the discontinued LLVM Phabricator instance.

Differential D79409

[InstCombine] Remove hasNoInfs check for pow(C,y) -> exp2(log2(C)*y)
ClosedPublic

Authored by foad on May 5 2020, 5:10 AM.

Details

Reviewers
xbolva00
spatel
efriedma
evandro
arsenm
Commits
rG9bc989a48d63: [InstCombine] Remove hasNoInfs check for pow(C,y) -> exp2(log2(C)*y)
Summary

We already check hasNoNaNs and that x is finite and strictly positive.
That only leaves the following special cases (taken from the Linux man
page for pow):

If the absolute value of x is less than 1, and y is negative infinity, the result is positive infinity.
If the absolute value of x is greater than 1, and y is negative infinity, the result is +0.
If the absolute value of x is less than 1, and y is positive infinity, the result is +0.
If the absolute value of x is greater than 1, and y is positive infinity, the result is positive infinity.

The transformation preserves all of these, so there is no need to limit
it to hasNoInfs.

Diff Detail

Unit TestsFailed

TimeTest
80 msLLVM.tools/llvm-isel-fuzzer::Unknown Unit Message ("")
50 msLLVM.tools/llvm-isel-fuzzer::Unknown Unit Message ("")
750 msLLVM.tools/llvm-isel-fuzzer::Unknown Unit Message ("")
760 msLLVM.tools/llvm-isel-fuzzer::Unknown Unit Message ("")
100 msLLVM.tools/llvm-isel-fuzzer::Unknown Unit Message ("")
View Full Test Results (11 Failed)

Event Timeline

foad created this revision.May 5 2020, 5:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2020, 5:10 AM
Herald added subscribers: llvm-commits, hiraditya. · View Herald Transcript
Harbormaster failed remote builds in B55772: Diff 262072!May 5 2020, 6:25 AM
foad added a reviewer: arsenm.May 5 2020, 8:23 AM
Herald added a subscriber: wdng. · View Herald TranscriptMay 5 2020, 8:23 AM
evandro added a comment.May 6 2020, 12:51 PM

pow(∞, y) → 0 if y < 0 or ∞ if y > 0
exp2(log2(∞) * y) → exp2(∞ * y) → exp2(∞) → 0 if y < 0 or ∞ if y > 0

foad added a comment.May 6 2020, 1:35 PM
In D79409#2023511, @evandro wrote:

pow(∞, y) → 0 if y < 0 or ∞ if y > 0
exp2(log2(∞) * y) → exp2(∞ * y) → exp2(∞) → 0 if y < 0 or ∞ if y > 0

We check that x is a finite constant before doing the transformation. It's only y that could be infinite.

evandro added a comment.May 6 2020, 2:10 PM

pow(x, -∞) → ∞ if |x| < 1 or 0 if |x| > 1
pow(x, +∞) → 0 if |x| < 1 or ∞ if |x| > 1
exp2(log2(x) * -∞) → exp2(±∞) → ∞ if x < 2 or 0 if x > 2
exp2(log2(x) * +∞) → exp2(±∞) → 0 if x < 2 or ∞ if x > 2

foad added a comment.May 6 2020, 2:13 PM
In D79409#2023671, @evandro wrote:

exp2(log2(x) * -∞) → exp2(±∞) → ∞ if x < 2 or 0 if x > 2
exp2(log2(x) * +∞) → exp2(±∞) → 0 if x < 2 or ∞ if x > 2

No, the sign of log2(x) depends on whether x is less or greater than 1, not less or greater than 2.

evandro added a comment.May 6 2020, 4:32 PM
In D79409#2023674, @foad wrote:

No, the sign of log2(x) depends on whether x is less or greater than 1, not less or greater than 2.

You're right.

pow(x, -∞) → ∞ if x < 1 or 0 if x > 1
pow(x, +∞) → 0 if x < 1 or ∞ if x > 1

exp2(log2(x) * -∞) → exp2(±∞) → ∞ if x < 1 or 0 if x > 1
exp2(log2(x) * +∞) → exp2(±∞) → 0 if x < 1 or ∞ if x > 1

Therefore the identity holds.

foad added a comment.May 7 2020, 1:52 AM

Thanks for the careful checking. Actually there is one problematic case:
pow(1, ±∞) → 1
exp2(log2(1) * ±∞) → exp2(0 * ±∞) → exp2(NaN) → NaN
I guess we never hit this case in practice because it will have been simplified elsewhere, but I will add something to the patch to handle it.

foad updated this revision to Diff 262606.May 7 2020, 4:02 AM

Rebase. Check for x == 1.

spatel added inline comments.May 7 2020, 5:06 AM
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
1688–1689

There's no way for Base to be 1.0 after here, so we can use an assert rather than 'if' clause in the new code.

foad updated this revision to Diff 262623.May 7 2020, 5:42 AM

Assert.

foad updated this revision to Diff 262625.May 7 2020, 5:45 AM

Fix silly mistake in last revision.

foad marked an inline comment as done.May 7 2020, 5:57 AM
Harbormaster failed remote builds in B56046: Diff 262606!May 7 2020, 6:12 AM
Harbormaster failed remote builds in B56054: Diff 262623!May 7 2020, 7:20 AM
Harbormaster failed remote builds in B56055: Diff 262625!May 7 2020, 7:54 AM
evandro added inline comments.May 8 2020, 1:50 PM
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
1570

This comment makes more sense immediately before the assert(). Personally, it could also be shorter.

foad updated this revision to Diff 263015.May 9 2020, 5:59 AM

Move and shorten the comment.

Harbormaster completed remote builds in B56232: Diff 263015.May 9 2020, 7:57 AM
foad added a comment.May 19 2020, 5:31 AM

Ping!

spatel accepted this revision.May 19 2020, 6:36 AM

LGTM

This revision is now accepted and ready to land.May 19 2020, 6:36 AM
Closed by commit rG9bc989a48d63: [InstCombine] Remove hasNoInfs check for pow(C,y) -> exp2(log2(C)*y) (authored by foad). · Explain WhyMay 19 2020, 9:17 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
lib/
Transforms/
Utils/
10 lines
test/
Transforms/
InstCombine/
5 lines

Diff 262606

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Show First 20 Lines • Show All 1,557 Lines • ▼ Show 20 LinesValue *LibCallSimplifier::replacePowWithExp(CallInst *Pow, IRBuilderBase &B) {
// pow(10.0, x) -> exp10(x) // pow(10.0, x) -> exp10(x)
// TODO: There is no exp10() intrinsic yet, but some day there shall be one. // TODO: There is no exp10() intrinsic yet, but some day there shall be one.
if (match(Base, m_SpecificFP(10.0)) && if (match(Base, m_SpecificFP(10.0)) &&
hasFloatFn(TLI, Ty, LibFunc_exp10, LibFunc_exp10f, LibFunc_exp10l)) hasFloatFn(TLI, Ty, LibFunc_exp10, LibFunc_exp10f, LibFunc_exp10l))
return emitUnaryFloatFnCall(Expo, TLI, LibFunc_exp10, LibFunc_exp10f, return emitUnaryFloatFnCall(Expo, TLI, LibFunc_exp10, LibFunc_exp10f,
LibFunc_exp10l, B, Attrs); LibFunc_exp10l, B, Attrs);
// pow(n, x) -> exp2(log2(n) * x) // pow(x, y) -> exp2(log2(x) * y)
if (Pow->hasApproxFunc() && Pow->hasNoNaNs() && Pow->hasNoInfs() && // We can do this transformation for any positive finite n except 1.0, because
BaseF->isFiniteNonZero() && !BaseF->isNegative()) { // pow has this behaviour:
// If x is +1, the result is 1.0 (even if y is a NaN).
// which is hard to preserve if y might be infinite or NaN.
evandroUnsubmitted
Not Done
ReplyInline Actions

This comment makes more sense immediately before the assert(). Personally, it could also be shorter.

evandro: This comment makes more sense immediately before the `assert()`. Personally, it could also be…
if (Pow->hasApproxFunc() && Pow->hasNoNaNs() && BaseF->isFiniteNonZero() &&
!BaseF->isNegative() && !match(Base, m_FPOne())) {
Value *Log = nullptr; Value *Log = nullptr;
if (Ty->isFloatTy()) if (Ty->isFloatTy())
Log = ConstantFP::get(Ty, std::log2(BaseF->convertToFloat())); Log = ConstantFP::get(Ty, std::log2(BaseF->convertToFloat()));
else if (Ty->isDoubleTy()) else if (Ty->isDoubleTy())
Log = ConstantFP::get(Ty, std::log2(BaseF->convertToDouble())); Log = ConstantFP::get(Ty, std::log2(BaseF->convertToDouble()));
if (Log) { if (Log) {
Value *FMul = B.CreateFMul(Log, Expo, "mul"); Value *FMul = B.CreateFMul(Log, Expo, "mul");
▲ Show 20 Lines • Show All 99 Lines • ▼ Show 20 LinesValue *LibCallSimplifier::optimizePow(CallInst *Pow, IRBuilderBase &B) {
// unless the result is expected to be double precision. // unless the result is expected to be double precision.
if (UnsafeFPShrink && Name == TLI->getName(LibFunc_pow) && if (UnsafeFPShrink && Name == TLI->getName(LibFunc_pow) &&
hasFloatVersion(Name)) hasFloatVersion(Name))
Shrunk = optimizeBinaryDoubleFP(Pow, B, true); Shrunk = optimizeBinaryDoubleFP(Pow, B, true);
// Evaluate special cases related to the base. // Evaluate special cases related to the base.
// pow(1.0, x) -> 1.0 // pow(1.0, x) -> 1.0
if (match(Base, m_FPOne())) if (match(Base, m_FPOne()))
return Base; return Base;
spatelUnsubmitted
Done
ReplyInline Actions

There's no way for Base to be 1.0 after here, so we can use an assert rather than 'if' clause in the new code.

spatel: There's no way for Base to be 1.0 after here, so we can use an assert rather than 'if' clause…
if (Value *Exp = replacePowWithExp(Pow, B)) if (Value *Exp = replacePowWithExp(Pow, B))
return Exp; return Exp;
// Evaluate special cases related to the exponent. // Evaluate special cases related to the exponent.
// pow(x, -1.0) -> 1.0 / x // pow(x, -1.0) -> 1.0 / x
if (match(Expo, m_SpecificFP(-1.0))) if (match(Expo, m_SpecificFP(-1.0)))
▲ Show 20 Lines • Show All 1,779 Lines • Show Last 20 Lines

llvm/test/Transforms/InstCombine/pow-exp.ll

Show First 20 Lines • Show All 394 Lines • ▼ Show 20 Lines
; CHECK-NEXT: ret double [[CALL]] ; CHECK-NEXT: ret double [[CALL]]
; ;
%call = tail call afn ninf double @pow(double 0x3FE6666666666666, double %e) %call = tail call afn ninf double @pow(double 0x3FE6666666666666, double %e)
ret double %call ret double %call
} }
define double @pow_ok_base_no_ninf(double %e) { define double @pow_ok_base_no_ninf(double %e) {
; CHECK-LABEL: @pow_ok_base_no_ninf( ; CHECK-LABEL: @pow_ok_base_no_ninf(
; CHECK-NEXT: [[CALL:%.*]] = tail call nnan afn double @pow(double 0x3FE6666666666666, double [[E:%.*]]) ; CHECK-NEXT: [[MUL:%.*]] = fmul nnan afn double [[E:%.*]], 0xBFE0776{{.*}}
; CHECK-NEXT: ret double [[CALL]] ; CHECK-NEXT: [[EXP2:%.*]] = call nnan afn double @exp2(double [[MUL]])
; CHECK-NEXT: ret double [[EXP2]]
; ;
%call = tail call afn nnan double @pow(double 0x3FE6666666666666, double %e) %call = tail call afn nnan double @pow(double 0x3FE6666666666666, double %e)
ret double %call ret double %call
} }
define float @powf_zero_base(float %e) { define float @powf_zero_base(float %e) {
; CHECK-LABEL: @powf_zero_base( ; CHECK-LABEL: @powf_zero_base(
; CHECK-NEXT: [[CALL:%.*]] = tail call nnan ninf afn float @powf(float 0.000000e+00, float [[E:%.*]]) ; CHECK-NEXT: [[CALL:%.*]] = tail call nnan ninf afn float @powf(float 0.000000e+00, float [[E:%.*]])
▲ Show 20 Lines • Show All 87 Lines • Show Last 20 Lines