From a01a0c820547def704b21a640547133d053dcd54 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Sun, 9 Jun 2019 16:22:01 +0000 Subject: [PATCH] [InstCombine] change canonicalization to fabs() to use FMF on fneg 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 fneg (fsub) because they have the same operand. This works around the most glaring FMF logical inconsistency cited in PR38086: https://bugs.llvm.org/show_bug.cgi?id=38086 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362909 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineSelect.cpp | 38 ++++++++---- test/Transforms/InstCombine/fabs.ll | 60 +++++++++---------- 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineSelect.cpp b/lib/Transforms/InstCombine/InstCombineSelect.cpp index 5a14726bee2..aeeab4788f2 100644 --- a/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1883,22 +1883,34 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) { Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, FCI); return replaceInstUsesWith(SI, Fabs); } - // With nsz: - // (X < +/-0.0) ? -X : X --> fabs(X) - // (X <= +/-0.0) ? -X : X --> fabs(X) - // (X > +/-0.0) ? X : -X --> fabs(X) - // (X >= +/-0.0) ? X : -X --> fabs(X) - if (FCI->hasNoSignedZeros() && - ((X == FalseVal && match(TrueVal, m_FNeg(m_Specific(X))) && - (Pred == FCmpInst::FCMP_OLT || Pred == FCmpInst::FCMP_OLE)) || - (X == TrueVal && match(FalseVal, m_FNeg(m_Specific(X))) && - (Pred == FCmpInst::FCMP_OGT || Pred == FCmpInst::FCMP_OGE)))) { - 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. + // With nnan and nsz: + // (X < +/-0.0) ? -X : X --> fabs(X) + // (X <= +/-0.0) ? -X : X --> fabs(X) + Instruction *FNeg; + if (match(CondVal, m_FCmp(Pred, m_Specific(FalseVal), m_AnyZeroFP())) && + match(TrueVal, m_FNeg(m_Specific(FalseVal))) && + match(TrueVal, m_Instruction(FNeg)) && + FNeg->hasNoNaNs() && FNeg->hasNoSignedZeros() && + (Pred == FCmpInst::FCMP_OLT || Pred == FCmpInst::FCMP_OLE)) { + Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, FalseVal, FNeg); + return replaceInstUsesWith(SI, Fabs); + } + // With nnan and nsz: + // (X > +/-0.0) ? X : -X --> fabs(X) + // (X >= +/-0.0) ? X : -X --> fabs(X) + if (match(CondVal, m_FCmp(Pred, m_Specific(TrueVal), m_AnyZeroFP())) && + match(FalseVal, m_FNeg(m_Specific(TrueVal))) && + match(FalseVal, m_Instruction(FNeg)) && + FNeg->hasNoNaNs() && FNeg->hasNoSignedZeros() && + (Pred == FCmpInst::FCMP_OGT || Pred == FCmpInst::FCMP_OGE)) { + Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, TrueVal, FNeg); + return replaceInstUsesWith(SI, Fabs); + } + // See if we are selecting two values based on a comparison of the two values. if (ICmpInst *ICI = dyn_cast(CondVal)) if (Instruction *Result = foldSelectInstWithICmp(SI, ICI)) diff --git a/test/Transforms/InstCombine/fabs.ll b/test/Transforms/InstCombine/fabs.ll index ef4fc25dff1..9962c02d137 100644 --- a/test/Transforms/InstCombine/fabs.ll +++ b/test/Transforms/InstCombine/fabs.ll @@ -321,8 +321,8 @@ define double @select_fcmp_nnan_nsz_olt_zero(double %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz double @llvm.fabs.f64(double [[X:%.*]]) ; CHECK-NEXT: ret double [[TMP1]] ; - %ltzero = fcmp nnan nsz olt double %x, 0.0 - %negx = fsub double -0.0, %x + %ltzero = fcmp olt double %x, 0.0 + %negx = fsub nnan nsz double -0.0, %x %fabs = select i1 %ltzero, double %negx, double %x ret double %fabs } @@ -332,8 +332,8 @@ define double @select_fcmp_nnan_nsz_olt_zero_unary_fneg(double %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz double @llvm.fabs.f64(double [[X:%.*]]) ; CHECK-NEXT: ret double [[TMP1]] ; - %ltzero = fcmp nnan nsz olt double %x, 0.0 - %negx = fneg double %x + %ltzero = fcmp olt double %x, 0.0 + %negx = fneg nnan nsz double %x %fabs = select i1 %ltzero, double %negx, double %x ret double %fabs } @@ -345,8 +345,8 @@ define float @select_fcmp_nnan_nsz_olt_negzero(float %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan ninf nsz float @llvm.fabs.f32(float [[X:%.*]]) ; CHECK-NEXT: ret float [[TMP1]] ; - %ltzero = fcmp nnan nsz ninf olt float %x, -0.0 - %negx = fsub float -0.0, %x + %ltzero = fcmp olt float %x, -0.0 + %negx = fsub nnan ninf nsz float -0.0, %x %fabs = select i1 %ltzero, float %negx, float %x ret float %fabs } @@ -356,8 +356,8 @@ define float @select_fcmp_nnan_nsz_olt_negzero_unary_fneg(float %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan ninf nsz float @llvm.fabs.f32(float [[X:%.*]]) ; CHECK-NEXT: ret float [[TMP1]] ; - %ltzero = fcmp nnan nsz ninf olt float %x, -0.0 - %negx = fneg float %x + %ltzero = fcmp olt float %x, -0.0 + %negx = fneg nnan ninf nsz float %x %fabs = select i1 %ltzero, float %negx, float %x ret float %fabs } @@ -369,8 +369,8 @@ define double @select_fcmp_nnan_nsz_ole_zero(double %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call fast double @llvm.fabs.f64(double [[X:%.*]]) ; CHECK-NEXT: ret double [[TMP1]] ; - %lezero = fcmp fast ole double %x, 0.0 - %negx = fsub double -0.0, %x + %lezero = fcmp ole double %x, 0.0 + %negx = fsub fast double -0.0, %x %fabs = select i1 %lezero, double %negx, double %x ret double %fabs } @@ -380,8 +380,8 @@ define double @select_fcmp_nnan_nsz_ole_zero_unary_fneg(double %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call fast double @llvm.fabs.f64(double [[X:%.*]]) ; CHECK-NEXT: ret double [[TMP1]] ; - %lezero = fcmp fast ole double %x, 0.0 - %negx = fneg double %x + %lezero = fcmp ole double %x, 0.0 + %negx = fneg fast double %x %fabs = select i1 %lezero, double %negx, double %x ret double %fabs } @@ -393,8 +393,8 @@ define float @select_fcmp_nnan_nsz_ole_negzero(float %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz float @llvm.fabs.f32(float [[X:%.*]]) ; CHECK-NEXT: ret float [[TMP1]] ; - %lezero = fcmp nnan nsz ole float %x, -0.0 - %negx = fsub float -0.0, %x + %lezero = fcmp ole float %x, -0.0 + %negx = fsub nnan nsz float -0.0, %x %fabs = select i1 %lezero, float %negx, float %x ret float %fabs } @@ -404,8 +404,8 @@ define float @select_fcmp_nnan_nsz_ole_negzero_unary_fneg(float %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz float @llvm.fabs.f32(float [[X:%.*]]) ; CHECK-NEXT: ret float [[TMP1]] ; - %lezero = fcmp nnan nsz ole float %x, -0.0 - %negx = fneg float %x + %lezero = fcmp ole float %x, -0.0 + %negx = fneg nnan nsz float %x %fabs = select i1 %lezero, float %negx, float %x ret float %fabs } @@ -417,8 +417,8 @@ define <2 x float> @select_fcmp_nnan_nsz_ogt_zero(<2 x float> %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz arcp <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]]) ; CHECK-NEXT: ret <2 x float> [[TMP1]] ; - %gtzero = fcmp nnan nsz arcp ogt <2 x float> %x, zeroinitializer - %negx = fsub <2 x float> , %x + %gtzero = fcmp ogt <2 x float> %x, zeroinitializer + %negx = fsub nnan nsz arcp <2 x float> , %x %fabs = select <2 x i1> %gtzero, <2 x float> %x, <2 x float> %negx ret <2 x float> %fabs } @@ -428,8 +428,8 @@ define <2 x float> @select_fcmp_nnan_nsz_ogt_zero_unary_fneg(<2 x float> %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz arcp <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]]) ; CHECK-NEXT: ret <2 x float> [[TMP1]] ; - %gtzero = fcmp nnan nsz arcp ogt <2 x float> %x, zeroinitializer - %negx = fneg <2 x float> %x + %gtzero = fcmp ogt <2 x float> %x, zeroinitializer + %negx = fneg nnan nsz arcp <2 x float> %x %fabs = select <2 x i1> %gtzero, <2 x float> %x, <2 x float> %negx ret <2 x float> %fabs } @@ -441,8 +441,8 @@ define half @select_fcmp_nnan_nsz_ogt_negzero(half %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call fast half @llvm.fabs.f16(half [[X:%.*]]) ; CHECK-NEXT: ret half [[TMP1]] ; - %gtzero = fcmp fast ogt half %x, -0.0 - %negx = fsub half 0.0, %x + %gtzero = fcmp ogt half %x, -0.0 + %negx = fsub fast half 0.0, %x %fabs = select i1 %gtzero, half %x, half %negx ret half %fabs } @@ -454,8 +454,8 @@ define <2 x double> @select_fcmp_nnan_nsz_oge_zero(<2 x double> %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call reassoc nnan nsz <2 x double> @llvm.fabs.v2f64(<2 x double> [[X:%.*]]) ; CHECK-NEXT: ret <2 x double> [[TMP1]] ; - %gezero = fcmp nnan nsz reassoc oge <2 x double> %x, zeroinitializer - %negx = fsub <2 x double> , %x + %gezero = fcmp oge <2 x double> %x, zeroinitializer + %negx = fsub nnan nsz reassoc <2 x double> , %x %fabs = select <2 x i1> %gezero, <2 x double> %x, <2 x double> %negx ret <2 x double> %fabs } @@ -465,8 +465,8 @@ define <2 x double> @select_fcmp_nnan_nsz_oge_zero_unary_fneg(<2 x double> %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call reassoc nnan nsz <2 x double> @llvm.fabs.v2f64(<2 x double> [[X:%.*]]) ; CHECK-NEXT: ret <2 x double> [[TMP1]] ; - %gezero = fcmp nnan nsz reassoc oge <2 x double> %x, zeroinitializer - %negx = fneg <2 x double> %x + %gezero = fcmp oge <2 x double> %x, zeroinitializer + %negx = fneg nnan nsz reassoc <2 x double> %x %fabs = select <2 x i1> %gezero, <2 x double> %x, <2 x double> %negx ret <2 x double> %fabs } @@ -478,8 +478,8 @@ define half @select_fcmp_nnan_nsz_oge_negzero(half %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz half @llvm.fabs.f16(half [[X:%.*]]) ; CHECK-NEXT: ret half [[TMP1]] ; - %gezero = fcmp nnan nsz oge half %x, -0.0 - %negx = fsub half -0.0, %x + %gezero = fcmp oge half %x, -0.0 + %negx = fsub nnan nsz half -0.0, %x %fabs = select i1 %gezero, half %x, half %negx ret half %fabs } @@ -489,8 +489,8 @@ define half @select_fcmp_nnan_nsz_oge_negzero_unary_fneg(half %x) { ; CHECK-NEXT: [[TMP1:%.*]] = call nnan nsz half @llvm.fabs.f16(half [[X:%.*]]) ; CHECK-NEXT: ret half [[TMP1]] ; - %gezero = fcmp nnan nsz oge half %x, -0.0 - %negx = fneg half %x + %gezero = fcmp oge half %x, -0.0 + %negx = fneg nnan nsz half %x %fabs = select i1 %gezero, half %x, half %negx ret half %fabs } -- 2.40.0