[llvm] r266175 - [InstCombine] We folded an fcmp to an i1 instead of a vector of i1
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 18 02:52:51 PDT 2016
Thanks!
/Mikael
On 04/15/2016 07:26 PM, David Majnemer wrote:
> Fixed in r266452.
>
> On Fri, Apr 15, 2016 at 8:07 AM, David Majnemer
> <david.majnemer at gmail.com <mailto:david.majnemer at gmail.com>> wrote:
>
> Thanks, I'll take a look ASAP.
>
>
> On Friday, April 15, 2016, Mikael Holmén <mikael.holmen at ericsson.com
> <mailto:mikael.holmen at ericsson.com>> wrote:
>
> Hi David,
>
> With this commit the following code crashes instcombine:
>
> declare float @fabsf()
>
> define void @foo() {
> %_tmp4 = call float @fabsf()
> store float %_tmp4, float* undef
> %_tmp5 = load float, float* undef
> %_tmp6 = fcmp olt float %_tmp5, 0.000000e+00
> br i1 %_tmp6, label %bb_lslct16, label %bb_lslct16
>
> bb_lslct16: ; preds = %0, %0
> ret void
> }
>
> build-all/bin/opt -S -instcombine crash.ll
> handled by SimplifyFCmpInst
> UNREACHABLE executed at
> ../lib/Transforms/InstCombine/InstCombineCompares.cpp:4582!
>
> --debug printouts:
>
> Args: build-all/bin/opt -S -instcombine crash.ll --debug
>
>
> INSTCOMBINE ITERATION #1 on foo
> IC: ADDING: 6 instrs to worklist
> IC: Visiting: %_tmp4 = call float @fabsf()
> IC: Visiting: store float %_tmp4, float* undef
> IC: Visiting: %_tmp5 = load float, float* undef
> IC: Replacing %_tmp5 = load float, float* undef, align 4
> with %_tmp4 = call float @fabsf()
> IC: Mod = %_tmp5 = load float, float* undef
> New = %_tmp5 = load float, float* undef, align 4
> IC: ERASE %_tmp5 = load float, float* undef, align 4
> IC: Visiting: %_tmp6 = fcmp olt float %_tmp4, 0.000000e+00
> handled by SimplifyFCmpInst
> UNREACHABLE executed at
> ../lib/Transforms/InstCombine/InstCombineCompares.cpp:4582!
>
> Regards,
> Mikael
>
> On 04/13/2016 08:55 AM, David Majnemer via llvm-commits wrote:
>
> Author: majnemer
> Date: Wed Apr 13 01:55:52 2016
> New Revision: 266175
>
> URL: http://llvm.org/viewvc/llvm-project?rev=266175&view=rev
> Log:
> [InstCombine] We folded an fcmp to an i1 instead of a vector
> of i1
>
> Remove an ad-hoc transform in InstCombine and replace it
> with more
> general machinery (ValueTracking, InstructionSimplify and
> VectorUtils).
>
> This fixes PR27332.
>
> Added:
> llvm/trunk/test/Transforms/InstCombine/pr27332.ll
> Modified:
> llvm/trunk/include/llvm/Analysis/ValueTracking.h
> llvm/trunk/include/llvm/Analysis/VectorUtils.h
> llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> llvm/trunk/lib/Analysis/ValueTracking.cpp
> llvm/trunk/lib/Analysis/VectorUtils.cpp
>
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>
> llvm/trunk/test/Transforms/InstCombine/zero-point-zero-add.ll
>
> Modified: llvm/trunk/include/llvm/Analysis/ValueTracking.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ValueTracking.h?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/ValueTracking.h (original)
> +++ llvm/trunk/include/llvm/Analysis/ValueTracking.h Wed Apr
> 13 01:55:52 2016
> @@ -145,12 +145,14 @@ namespace llvm {
> /// CannotBeNegativeZero - Return true if we can prove
> that the specified FP
> /// value is never equal to -0.0.
> ///
> - bool CannotBeNegativeZero(const Value *V, unsigned Depth
> = 0);
> + bool CannotBeNegativeZero(const Value *V, const
> TargetLibraryInfo *TLI,
> + unsigned Depth = 0);
>
> /// CannotBeOrderedLessThanZero - Return true if we can
> prove that the
> /// specified FP value is either a NaN or never less
> than 0.0.
> ///
> - bool CannotBeOrderedLessThanZero(const Value *V, unsigned
> Depth = 0);
> + bool CannotBeOrderedLessThanZero(const Value *V, const
> TargetLibraryInfo *TLI,
> + unsigned Depth = 0);
>
> /// isBytewiseValue - If the specified value can be set
> by repeating the same
> /// byte in memory, return the i8 value that it is
> represented with. This is
>
> Modified: llvm/trunk/include/llvm/Analysis/VectorUtils.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/VectorUtils.h?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/VectorUtils.h (original)
> +++ llvm/trunk/include/llvm/Analysis/VectorUtils.h Wed Apr
> 13 01:55:52 2016
> @@ -59,7 +59,8 @@ Intrinsic::ID checkBinaryFloatSignature(
> /// \brief Returns intrinsic ID for call.
> /// For the input call instruction it finds mapping
> intrinsic and returns
> /// its intrinsic ID, in case it does not found it return
> not_intrinsic.
> -Intrinsic::ID getIntrinsicIDForCall(CallInst *CI, const
> TargetLibraryInfo *TLI);
> +Intrinsic::ID getIntrinsicIDForCall(const CallInst *CI,
> + const TargetLibraryInfo
> *TLI);
>
> /// \brief Find the operand of the GEP that should be
> checked for consecutive
> /// stores. This ignores trailing indices that have no
> effect on the final
>
> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Wed Apr
> 13 01:55:52 2016
> @@ -794,7 +794,7 @@ static Value *SimplifyFAddInst(Value *Op
>
> // fadd X, 0 ==> X, when we know X is not -0
> if (match(Op1, m_Zero()) &&
> - (FMF.noSignedZeros() || CannotBeNegativeZero(Op0)))
> + (FMF.noSignedZeros() || CannotBeNegativeZero(Op0,
> Q.TLI)))
> return Op0;
>
> // fadd [nnan ninf] X, (fsub [nnan ninf] 0, X) ==> 0
> @@ -830,7 +830,7 @@ static Value *SimplifyFSubInst(Value *Op
>
> // fsub X, -0 ==> X, when we know X is not -0
> if (match(Op1, m_NegZero()) &&
> - (FMF.noSignedZeros() || CannotBeNegativeZero(Op0)))
> + (FMF.noSignedZeros() || CannotBeNegativeZero(Op0,
> Q.TLI)))
> return Op0;
>
> // fsub -0.0, (fsub -0.0, X) ==> X
> @@ -3112,7 +3112,14 @@ static Value *SimplifyFCmpInst(unsigned
> }
>
> // Handle fcmp with constant RHS
> - if (ConstantFP *CFP = dyn_cast<ConstantFP>(RHS)) {
> + const ConstantFP *CFP = nullptr;
> + if (const auto *RHSC = dyn_cast<Constant>(RHS)) {
> + if (RHS->getType()->isVectorTy())
> + CFP =
> dyn_cast_or_null<ConstantFP>(RHSC->getSplatValue());
> + else
> + CFP = dyn_cast<ConstantFP>(RHSC);
> + }
> + if (CFP) {
> // If the constant is a nan, see if we can fold the
> comparison based on it.
> if (CFP->getValueAPF().isNaN()) {
> if (FCmpInst::isOrdered(Pred)) // True "if ordered
> and foo"
> @@ -3120,7 +3127,7 @@ static Value *SimplifyFCmpInst(unsigned
> assert(FCmpInst::isUnordered(Pred) &&
> "Comparison must be either ordered or
> unordered!");
> // True if unordered.
> - return ConstantInt::getTrue(CFP->getContext());
> + return ConstantInt::get(GetCompareTy(LHS), 1);
> }
> // Check whether the constant is an infinity.
> if (CFP->getValueAPF().isInfinity()) {
> @@ -3128,10 +3135,10 @@ static Value *SimplifyFCmpInst(unsigned
> switch (Pred) {
> case FCmpInst::FCMP_OLT:
> // No value is ordered and less than negative
> infinity.
> - return ConstantInt::getFalse(CFP->getContext());
> + return ConstantInt::get(GetCompareTy(LHS), 0);
> case FCmpInst::FCMP_UGE:
> // All values are unordered with or at least
> negative infinity.
> - return ConstantInt::getTrue(CFP->getContext());
> + return ConstantInt::get(GetCompareTy(LHS), 1);
> default:
> break;
> }
> @@ -3139,10 +3146,10 @@ static Value *SimplifyFCmpInst(unsigned
> switch (Pred) {
> case FCmpInst::FCMP_OGT:
> // No value is ordered and greater than infinity.
> - return ConstantInt::getFalse(CFP->getContext());
> + return ConstantInt::get(GetCompareTy(LHS), 0);
> case FCmpInst::FCMP_ULE:
> // All values are unordered with and at most
> infinity.
> - return ConstantInt::getTrue(CFP->getContext());
> + return ConstantInt::get(GetCompareTy(LHS), 1);
> default:
> break;
> }
> @@ -3151,13 +3158,13 @@ static Value *SimplifyFCmpInst(unsigned
> if (CFP->getValueAPF().isZero()) {
> switch (Pred) {
> case FCmpInst::FCMP_UGE:
> - if (CannotBeOrderedLessThanZero(LHS))
> - return ConstantInt::getTrue(CFP->getContext());
> + if (CannotBeOrderedLessThanZero(LHS, Q.TLI))
> + return ConstantInt::get(GetCompareTy(LHS), 1);
> break;
> case FCmpInst::FCMP_OLT:
> // X < 0
> - if (CannotBeOrderedLessThanZero(LHS))
> - return ConstantInt::getFalse(CFP->getContext());
> + if (CannotBeOrderedLessThanZero(LHS, Q.TLI))
> + return ConstantInt::get(GetCompareTy(LHS), 0);
> break;
> default:
> break;
>
> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Wed Apr 13
> 01:55:52 2016
> @@ -20,6 +20,7 @@
> #include "llvm/Analysis/MemoryBuiltins.h"
> #include "llvm/Analysis/Loads.h"
> #include "llvm/Analysis/LoopInfo.h"
> +#include "llvm/Analysis/VectorUtils.h"
> #include "llvm/IR/CallSite.h"
> #include "llvm/IR/ConstantRange.h"
> #include "llvm/IR/Constants.h"
> @@ -2267,7 +2268,8 @@ bool llvm::ComputeMultiple(Value *V, uns
> /// NOTE: this function will need to be revisited when we
> support non-default
> /// rounding modes!
> ///
> -bool llvm::CannotBeNegativeZero(const Value *V, unsigned
> Depth) {
> +bool llvm::CannotBeNegativeZero(const Value *V, const
> TargetLibraryInfo *TLI,
> + unsigned Depth) {
> if (const ConstantFP *CFP = dyn_cast<ConstantFP>(V))
> return !CFP->getValueAPF().isNegZero();
>
> @@ -2295,30 +2297,26 @@ bool llvm::CannotBeNegativeZero(const Va
> if (isa<SIToFPInst>(I) || isa<UIToFPInst>(I))
> return true;
>
> - if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
> + if (const CallInst *CI = dyn_cast<CallInst>(I)) {
> + Intrinsic::ID IID = getIntrinsicIDForCall(CI, TLI);
> + switch (IID) {
> + default:
> + break;
> // sqrt(-0.0) = -0.0, no other negative results are
> possible.
> - if (II->getIntrinsicID() == Intrinsic::sqrt)
> - return CannotBeNegativeZero(II->getArgOperand(0),
> Depth+1);
> -
> - if (const CallInst *CI = dyn_cast<CallInst>(I))
> - if (const Function *F = CI->getCalledFunction()) {
> - if (F->isDeclaration()) {
> - // abs(x) != -0.0
> - if (F->getName() == "abs") return true;
> - // fabs[lf](x) != -0.0
> - if (F->getName() == "fabs") return true;
> - if (F->getName() == "fabsf") return true;
> - if (F->getName() == "fabsl") return true;
> - if (F->getName() == "sqrt" || F->getName() ==
> "sqrtf" ||
> - F->getName() == "sqrtl")
> - return CannotBeNegativeZero(CI->getArgOperand(0),
> Depth+1);
> - }
> + case Intrinsic::sqrt:
> + return CannotBeNegativeZero(CI->getArgOperand(0),
> TLI, Depth + 1);
> + // fabs(x) != -0.0
> + case Intrinsic::fabs:
> + return true;
> }
> + }
>
> return false;
> }
>
> -bool llvm::CannotBeOrderedLessThanZero(const Value *V,
> unsigned Depth) {
> +bool llvm::CannotBeOrderedLessThanZero(const Value *V,
> + const
> TargetLibraryInfo *TLI,
> + unsigned Depth) {
> if (const ConstantFP *CFP = dyn_cast<ConstantFP>(V))
> return !CFP->getValueAPF().isNegative() ||
> CFP->getValueAPF().isZero();
>
> @@ -2344,43 +2342,44 @@ bool llvm::CannotBeOrderedLessThanZero(c
> case Instruction::FAdd:
> case Instruction::FDiv:
> case Instruction::FRem:
> - return CannotBeOrderedLessThanZero(I->getOperand(0),
> Depth+1) &&
> - CannotBeOrderedLessThanZero(I->getOperand(1),
> Depth+1);
> + return CannotBeOrderedLessThanZero(I->getOperand(0),
> TLI, Depth + 1) &&
> + CannotBeOrderedLessThanZero(I->getOperand(1),
> TLI, Depth + 1);
> case Instruction::Select:
> - return CannotBeOrderedLessThanZero(I->getOperand(1),
> Depth+1) &&
> - CannotBeOrderedLessThanZero(I->getOperand(2),
> Depth+1);
> + return CannotBeOrderedLessThanZero(I->getOperand(1),
> TLI, Depth + 1) &&
> + CannotBeOrderedLessThanZero(I->getOperand(2),
> TLI, Depth + 1);
> case Instruction::FPExt:
> case Instruction::FPTrunc:
> // Widening/narrowing never change sign.
> - return CannotBeOrderedLessThanZero(I->getOperand(0),
> Depth+1);
> - case Instruction::Call:
> - if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
> - switch (II->getIntrinsicID()) {
> - default: break;
> - case Intrinsic::maxnum:
> - return
> CannotBeOrderedLessThanZero(I->getOperand(0), Depth+1) ||
> -
> CannotBeOrderedLessThanZero(I->getOperand(1), Depth+1);
> - case Intrinsic::minnum:
> - return
> CannotBeOrderedLessThanZero(I->getOperand(0), Depth+1) &&
> -
> CannotBeOrderedLessThanZero(I->getOperand(1), Depth+1);
> - case Intrinsic::exp:
> - case Intrinsic::exp2:
> - case Intrinsic::fabs:
> - case Intrinsic::sqrt:
> - return true;
> - case Intrinsic::powi:
> - if (ConstantInt *CI =
> dyn_cast<ConstantInt>(I->getOperand(1))) {
> - // powi(x,n) is non-negative if n is even.
> - if (CI->getBitWidth() <= 64 && CI->getSExtValue()
> % 2u == 0)
> - return true;
> - }
> - return
> CannotBeOrderedLessThanZero(I->getOperand(0), Depth+1);
> - case Intrinsic::fma:
> - case Intrinsic::fmuladd:
> - // x*x+y is non-negative if y is non-negative.
> - return I->getOperand(0) == I->getOperand(1) &&
> -
> CannotBeOrderedLessThanZero(I->getOperand(2), Depth+1);
> + return CannotBeOrderedLessThanZero(I->getOperand(0),
> TLI, Depth + 1);
> + case Instruction::Call:
> + Intrinsic::ID IID =
> getIntrinsicIDForCall(cast<CallInst>(I), TLI);
> + switch (IID) {
> + default:
> + break;
> + case Intrinsic::maxnum:
> + return CannotBeOrderedLessThanZero(I->getOperand(0),
> TLI, Depth + 1) ||
> + CannotBeOrderedLessThanZero(I->getOperand(1),
> TLI, Depth + 1);
> + case Intrinsic::minnum:
> + return CannotBeOrderedLessThanZero(I->getOperand(0),
> TLI, Depth + 1) &&
> + CannotBeOrderedLessThanZero(I->getOperand(1),
> TLI, Depth + 1);
> + case Intrinsic::exp:
> + case Intrinsic::exp2:
> + case Intrinsic::fabs:
> + case Intrinsic::sqrt:
> + return true;
> + case Intrinsic::powi:
> + if (ConstantInt *CI =
> dyn_cast<ConstantInt>(I->getOperand(1))) {
> + // powi(x,n) is non-negative if n is even.
> + if (CI->getBitWidth() <= 64 && CI->getSExtValue() %
> 2u == 0)
> + return true;
> }
> + return CannotBeOrderedLessThanZero(I->getOperand(0),
> TLI, Depth + 1);
> + case Intrinsic::fma:
> + case Intrinsic::fmuladd:
> + // x*x+y is non-negative if y is non-negative.
> + return I->getOperand(0) == I->getOperand(1) &&
> + CannotBeOrderedLessThanZero(I->getOperand(2),
> TLI, Depth + 1);
> + }
> break;
> }
> return false;
>
> Modified: llvm/trunk/lib/Analysis/VectorUtils.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/VectorUtils.cpp?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/VectorUtils.cpp (original)
> +++ llvm/trunk/lib/Analysis/VectorUtils.cpp Wed Apr 13
> 01:55:52 2016
> @@ -121,10 +121,10 @@ llvm::checkBinaryFloatSignature(const Ca
> /// \brief Returns intrinsic ID for call.
> /// For the input call instruction it finds mapping
> intrinsic and returns
> /// its ID, in case it does not found it return
> not_intrinsic.
> -Intrinsic::ID llvm::getIntrinsicIDForCall(CallInst *CI,
> +Intrinsic::ID llvm::getIntrinsicIDForCall(const CallInst *CI,
> const
> TargetLibraryInfo *TLI) {
> // If we have an intrinsic call, check if it is
> trivially vectorizable.
> - if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(CI)) {
> + if (const auto *II = dyn_cast<IntrinsicInst>(CI)) {
> Intrinsic::ID ID = II->getIntrinsicID();
> if (isTriviallyVectorizable(ID) || ID ==
> Intrinsic::lifetime_start ||
> ID == Intrinsic::lifetime_end || ID ==
> Intrinsic::assume)
>
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> ---
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
> +++
> llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Wed
> Apr 13 01:55:52 2016
> @@ -4570,7 +4570,7 @@ Instruction *InstCombiner::visitFCmpInst
> break;
> // fabs(x) < 0 --> false
> case FCmpInst::FCMP_OLT:
> - return replaceInstUsesWith(I, Builder->getFalse());
> + llvm_unreachable("handled by SimplifyFCmpInst");
> // fabs(x) > 0 --> x != 0
> case FCmpInst::FCMP_OGT:
> return new FCmpInst(FCmpInst::FCMP_ONE,
> CI->getArgOperand(0), RHSC);
>
> Added: llvm/trunk/test/Transforms/InstCombine/pr27332.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pr27332.ll?rev=266175&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/pr27332.ll (added)
> +++ llvm/trunk/test/Transforms/InstCombine/pr27332.ll Wed
> Apr 13 01:55:52 2016
> @@ -0,0 +1,11 @@
> +; RUN: opt -instcombine -S -o - < %s | FileCheck %s
> +declare <4 x float> @llvm.fabs.v4f32(<4 x float>)
> +
> +define <4 x i1> @test1(<4 x float> %V) {
> +entry:
> + %abs = call <4 x float> @llvm.fabs.v4f32(<4 x float> %V)
> + %cmp = fcmp olt <4 x float> %abs, zeroinitializer
> + ret <4 x i1> %cmp
> +}
> +; CHECK-LABEL: define <4 x i1> @test1(
> +; CHECK: ret <4 x i1> zeroinitializer
>
> Modified:
> llvm/trunk/test/Transforms/InstCombine/zero-point-zero-add.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/zero-point-zero-add.ll?rev=266175&r1=266174&r2=266175&view=diff
> ==============================================================================
> ---
> llvm/trunk/test/Transforms/InstCombine/zero-point-zero-add.ll (original)
> +++
> llvm/trunk/test/Transforms/InstCombine/zero-point-zero-add.ll Wed
> Apr 13 01:55:52 2016
> @@ -1,7 +1,7 @@
> ; NOTE: Assertions have been autogenerated by
> update_test_checks.py
> ; RUN: opt < %s -instcombine -S | FileCheck %s
>
> -declare double @abs(double)
> +declare double @fabs(double) readonly
>
> define double @test(double %X) {
> ; CHECK-LABEL: @test(
> @@ -15,10 +15,10 @@ define double @test(double %X) {
>
> define double @test1(double %X) {
> ; CHECK-LABEL: @test1(
> -; CHECK-NEXT: [[Y:%.*]] = call double @abs(double %X)
> +; CHECK-NEXT: [[Y:%.*]] = call double @fabs(double %X)
> ; CHECK-NEXT: ret double [[Y]]
> ;
> - %Y = call double @abs(double %X)
> + %Y = call double @fabs(double %X)
> %Z = fadd double %Y, 0.0
> ret double %Z
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
More information about the llvm-commits
mailing list