From 2a44ab0eda79ebc30e16123e42336907bbafcae6 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 14 Mar 2018 21:23:27 +0000 Subject: [PATCH] [InstSimplify] fix folds for (0.0 - X) + X --> 0 (PR27151) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit As shown in: https://bugs.llvm.org/show_bug.cgi?id=27151 ...the existing fold could miscompile when X is NaN. The fold was also dependent on 'ninf' but that's not necessary. From IEEE-754 (with default rounding which we can assume for these opcodes): "When the sum of two operands with opposite signs (or the difference of two operands with like signs) is exactly zero, the sign of that sum (or difference) shall be +0...However, x + x = x − (−x) retains the same sign as x even when x is zero." git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@327575 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/InstructionSimplify.cpp | 24 ++++++++++-------------- test/Transforms/InstSimplify/fast-math.ll | 30 ++++++++++++------------------ 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/lib/Analysis/InstructionSimplify.cpp b/lib/Analysis/InstructionSimplify.cpp index a95a775ead6..2b8e23b9569 100644 --- a/lib/Analysis/InstructionSimplify.cpp +++ b/lib/Analysis/InstructionSimplify.cpp @@ -4168,20 +4168,16 @@ static Value *SimplifyFAddInst(Value *Op0, Value *Op1, FastMathFlags FMF, (FMF.noSignedZeros() || CannotBeNegativeZero(Op0, Q.TLI))) return Op0; - // fadd [nnan ninf] X, (fsub [nnan ninf] 0, X) ==> 0 - // where nnan and ninf have to occur at least once somewhere in this - // expression - Value *SubOp = nullptr; - if (match(Op1, m_FSub(m_AnyZero(), m_Specific(Op0)))) - SubOp = Op1; - else if (match(Op0, m_FSub(m_AnyZero(), m_Specific(Op1)))) - SubOp = Op0; - if (SubOp) { - Instruction *FSub = cast(SubOp); - if ((FMF.noNaNs() || FSub->hasNoNaNs()) && - (FMF.noInfs() || FSub->hasNoInfs())) - return Constant::getNullValue(Op0->getType()); - } + // With nnan: (+/-0.0 - X) + X --> 0.0 (and commuted variant) + // We don't have to explicitly exclude infinities (ninf): INF + -INF == NaN. + // Negative zeros are allowed because we always end up with positive zero: + // X = -0.0: (-0.0 - (-0.0)) + (-0.0) == ( 0.0) + (-0.0) == 0.0 + // X = -0.0: ( 0.0 - (-0.0)) + (-0.0) == ( 0.0) + (-0.0) == 0.0 + // X = 0.0: (-0.0 - ( 0.0)) + ( 0.0) == (-0.0) + ( 0.0) == 0.0 + // X = 0.0: ( 0.0 - ( 0.0)) + ( 0.0) == ( 0.0) + ( 0.0) == 0.0 + if (FMF.noNaNs() && (match(Op0, m_FSub(m_AnyZero(), m_Specific(Op1))) || + match(Op1, m_FSub(m_AnyZero(), m_Specific(Op0))))) + return ConstantFP::getNullValue(Op0->getType()); return nullptr; } diff --git a/test/Transforms/InstSimplify/fast-math.ll b/test/Transforms/InstSimplify/fast-math.ll index bf862933a9c..2a3086481f2 100644 --- a/test/Transforms/InstSimplify/fast-math.ll +++ b/test/Transforms/InstSimplify/fast-math.ll @@ -46,50 +46,47 @@ define float @no_mul_zero_3(float %a) { ret float %b } -; FIXME: -X + X --> 0.0 (with nnan on the fadd) +; -X + X --> 0.0 (with nnan on the fadd) define float @fadd_fnegx(float %x) { ; CHECK-LABEL: @fadd_fnegx( -; CHECK-NEXT: [[NEGX:%.*]] = fsub float -0.000000e+00, [[X:%.*]] -; CHECK-NEXT: [[R:%.*]] = fadd nnan float [[NEGX]], [[X]] -; CHECK-NEXT: ret float [[R]] +; CHECK-NEXT: ret float 0.000000e+00 ; %negx = fsub float -0.0, %x %r = fadd nnan float %negx, %x ret float %r } -; FIXME: X + -X --> 0.0 (with nnan on the fadd) +; X + -X --> 0.0 (with nnan on the fadd) define <2 x float> @fadd_fnegx_commute_vec(<2 x float> %x) { ; CHECK-LABEL: @fadd_fnegx_commute_vec( -; CHECK-NEXT: [[NEGX:%.*]] = fsub <2 x float> , [[X:%.*]] -; CHECK-NEXT: [[R:%.*]] = fadd nnan <2 x float> [[X]], [[NEGX]] -; CHECK-NEXT: ret <2 x float> [[R]] +; CHECK-NEXT: ret <2 x float> zeroinitializer ; %negx = fsub <2 x float> , %x %r = fadd nnan <2 x float> %x, %negx ret <2 x float> %r } -; FIXME: Could be NaN. ; https://bugs.llvm.org/show_bug.cgi?id=26958 ; https://bugs.llvm.org/show_bug.cgi?id=27151 define float @fadd_fneg_nan(float %x) { ; CHECK-LABEL: @fadd_fneg_nan( -; CHECK-NEXT: ret float 0.000000e+00 +; CHECK-NEXT: [[T:%.*]] = fsub nnan float -0.000000e+00, [[X:%.*]] +; CHECK-NEXT: [[COULD_BE_NAN:%.*]] = fadd ninf float [[T]], [[X]] +; CHECK-NEXT: ret float [[COULD_BE_NAN]] ; %t = fsub nnan float -0.0, %x %could_be_nan = fadd ninf float %t, %x ret float %could_be_nan } -; FIXME: Could be NaN. - define float @fadd_fneg_nan_commute(float %x) { ; CHECK-LABEL: @fadd_fneg_nan_commute( -; CHECK-NEXT: ret float 0.000000e+00 +; CHECK-NEXT: [[T:%.*]] = fsub nnan ninf float -0.000000e+00, [[X:%.*]] +; CHECK-NEXT: [[COULD_BE_NAN:%.*]] = fadd float [[X]], [[T]] +; CHECK-NEXT: ret float [[COULD_BE_NAN]] ; %t = fsub nnan ninf float -0.0, %x %could_be_nan = fadd float %x, %t @@ -118,15 +115,12 @@ define <2 x float> @fadd_fsub_nnan_ninf_commute_vec(<2 x float> %x) { ret <2 x float> %zero } -; FIXME: Do fold this. ; 'ninf' is not required because 'nnan' allows us to assume -; that X is not INF/-INF (adding opposite INFs would be NaN). +; that X is not INF or -INF (adding opposite INFs would be NaN). define float @fadd_fsub_nnan(float %x) { ; CHECK-LABEL: @fadd_fsub_nnan( -; CHECK-NEXT: [[SUB:%.*]] = fsub float 0.000000e+00, [[X:%.*]] -; CHECK-NEXT: [[ZERO:%.*]] = fadd nnan float [[SUB]], [[X]] -; CHECK-NEXT: ret float [[ZERO]] +; CHECK-NEXT: ret float 0.000000e+00 ; %sub = fsub float 0.0, %x %zero = fadd nnan float %sub, %x -- 2.11.0