[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
Fri Apr 15 03:43:15 PDT 2016
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