From dd29c50b321951952a6ed54a1d27c8f314b1284b Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 10 Jun 2019 14:46:36 +0000 Subject: [PATCH] [InstCombine] change canonicalization to fabs() to use FMF on fsub Similar to rL362909: This isn't the ideal fix (use FMF on the select), but it's still an improvement until we have better FMF propagation to selects and other FP math operators. I don't think there's much risk of regression from this change by not including the FMF on the fcmp any more. The nsz/nnan FMF should be the same on the fcmp and the fsub because they have the same operand. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362943 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/InstCombine/InstCombineSelect.cpp | 40 +++++++++++++----------- test/Transforms/InstCombine/fabs.ll | 16 +++++----- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineSelect.cpp b/lib/Transforms/InstCombine/InstCombineSelect.cpp index 13452866f61..ced2f10e937 100644 --- a/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1866,27 +1866,29 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) { // NOTE: if we wanted to, this is where to detect MIN/MAX } - - // Canonicalize select with fcmp to fabs(). -0.0 makes this tricky. We need - // fast-math-flags (nsz) or fsub with +0.0 (not fneg) for this to work. We - // also require nnan because we do not want to unintentionally change the - // sign of a NaN value. - Value *X = FCI->getOperand(0); - FCmpInst::Predicate Pred = FCI->getPredicate(); - if (match(FCI->getOperand(1), m_AnyZeroFP()) && FCI->hasNoNaNs()) { - // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X) - // (X > +/-0.0) ? X : (0.0 - X) --> fabs(X) - if ((X == FalseVal && Pred == FCmpInst::FCMP_OLE && - match(TrueVal, m_FSub(m_PosZeroFP(), m_Specific(X)))) || - (X == TrueVal && Pred == FCmpInst::FCMP_OGT && - match(FalseVal, m_FSub(m_PosZeroFP(), m_Specific(X))))) { - Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, FCI); - return replaceInstUsesWith(SI, Fabs); - } - } } - // FIXME: These folds should test/propagate FMF from the select, not the fneg. + // Canonicalize select with fcmp to fabs(). -0.0 makes this tricky. We need + // fast-math-flags (nsz) or fsub with +0.0 (not fneg) for this to work. We + // also require nnan because we do not want to unintentionally change the + // sign of a NaN value. + // FIXME: These folds should test/propagate FMF from the select, not the + // fsub or fneg. + // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X) + Instruction *FSub; + if (match(CondVal, m_FCmp(Pred, m_Specific(FalseVal), m_AnyZeroFP())) && + match(TrueVal, m_FSub(m_PosZeroFP(), m_Specific(FalseVal))) && + match(TrueVal, m_Instruction(FSub)) && FSub->hasNoNaNs()) { + Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, FalseVal, FSub); + return replaceInstUsesWith(SI, Fabs); + } + // (X > +/-0.0) ? X : (0.0 - X) --> fabs(X) + if (match(CondVal, m_FCmp(Pred, m_Specific(TrueVal), m_AnyZeroFP())) && + match(FalseVal, m_FSub(m_PosZeroFP(), m_Specific(TrueVal))) && + match(FalseVal, m_Instruction(FSub)) && FSub->hasNoNaNs()) { + Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, TrueVal, FSub); + return replaceInstUsesWith(SI, Fabs); + } // With nnan and nsz: // (X < +/-0.0) ? -X : X --> fabs(X) // (X <= +/-0.0) ? -X : X --> fabs(X) diff --git a/test/Transforms/InstCombine/fabs.ll b/test/Transforms/InstCombine/fabs.ll index 08ff0446bb6..0f4f4808cc3 100644 --- a/test/Transforms/InstCombine/fabs.ll +++ b/test/Transforms/InstCombine/fabs.ll @@ -269,8 +269,8 @@ define double @select_fcmp_nnan_ole_zero(double %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan double @llvm.fabs.f64(double [[X:%.*]]) ; CHECK-NEXT: ret double [[TMP1]] ; - %lezero = fcmp nnan ole double %x, 0.0 - %negx = fsub double 0.0, %x + %lezero = fcmp ole double %x, 0.0 + %negx = fsub nnan double 0.0, %x %fabs = select i1 %lezero, double %negx, double %x ret double %fabs } @@ -282,8 +282,8 @@ define <2 x float> @select_fcmp_nnan_ole_negzero(<2 x float> %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]]) ; CHECK-NEXT: ret <2 x float> [[TMP1]] ; - %lezero = fcmp nnan ole <2 x float> %x, - %negx = fsub <2 x float> , %x + %lezero = fcmp ole <2 x float> %x, + %negx = fsub nnan <2 x float> , %x %fabs = select <2 x i1> %lezero, <2 x float> %negx, <2 x float> %x ret <2 x float> %fabs } @@ -295,8 +295,8 @@ define fp128 @select_fcmp_nnan_ogt_zero(fp128 %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan fp128 @llvm.fabs.f128(fp128 [[X:%.*]]) ; CHECK-NEXT: ret fp128 [[TMP1]] ; - %gtzero = fcmp nnan ogt fp128 %x, zeroinitializer - %negx = fsub fp128 zeroinitializer, %x + %gtzero = fcmp ogt fp128 %x, zeroinitializer + %negx = fsub nnan fp128 zeroinitializer, %x %fabs = select i1 %gtzero, fp128 %x, fp128 %negx ret fp128 %fabs } @@ -308,8 +308,8 @@ define half @select_fcmp_nnan_ogt_negzero(half %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan half @llvm.fabs.f16(half [[X:%.*]]) ; CHECK-NEXT: ret half [[TMP1]] ; - %gtzero = fcmp nnan ogt half %x, -0.0 - %negx = fsub half 0.0, %x + %gtzero = fcmp ogt half %x, -0.0 + %negx = fsub nnan half 0.0, %x %fabs = select i1 %gtzero, half %x, half %negx ret half %fabs } -- 2.11.0