From bcd8ea0e2d4d3ef3dcb63079ecbd48b232ceb4f5 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Thu, 7 Feb 2019 21:12:01 +0000 Subject: [PATCH] [InstCombine] Fix crashing from (icmp (bitcast ([su]itofp X)), Y) This fixes a class of bugs introduced by D44367, which transforms various cases of icmp (bitcast ([su]itofp X)), Y to icmp X, Y. If the bitcast is between vector types with a different number of elements, the current code will produce bad IR along the lines of: icmp ..., <...>. This patch suppresses the transform if the bitcast changes the number of vector elements. Patch by: @AndrewScheidecker (Andrew Scheidecker) Differential Revision: https://reviews.llvm.org/D57871 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353467 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineCompares.cpp | 62 ++++++++++--------- .../InstCombine/cast-int-icmp-eq-0.ll | 56 +++++++++++++++++ 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp index 5a63d558de6..dc2672537aa 100644 --- a/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -2510,38 +2510,42 @@ static Instruction *foldICmpBitCast(ICmpInst &Cmp, if (!Bitcast) return nullptr; - // Zero-equality and sign-bit checks are preserved through sitofp + bitcast. - // FIXME: This needs to check that the bitcast does not change the number of - // elements in a vector. ICmpInst::Predicate Pred = Cmp.getPredicate(); Value *Op1 = Cmp.getOperand(1); Value *BCSrcOp = Bitcast->getOperand(0); - Value *X; - if (match(BCSrcOp, m_SIToFP(m_Value(X)))) { - // icmp eq (bitcast (sitofp X)), 0 --> icmp eq X, 0 - // icmp ne (bitcast (sitofp X)), 0 --> icmp ne X, 0 - // icmp slt (bitcast (sitofp X)), 0 --> icmp slt X, 0 - // icmp sgt (bitcast (sitofp X)), 0 --> icmp sgt X, 0 - if ((Pred == ICmpInst::ICMP_EQ || Pred == ICmpInst::ICMP_SLT || - Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_SGT) && - match(Op1, m_Zero())) - return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType())); - - // icmp slt (bitcast (sitofp X)), 1 --> icmp slt X, 1 - if (Pred == ICmpInst::ICMP_SLT && match(Op1, m_One())) - return new ICmpInst(Pred, X, ConstantInt::get(X->getType(), 1)); - - // icmp sgt (bitcast (sitofp X)), -1 --> icmp sgt X, -1 - if (Pred == ICmpInst::ICMP_SGT && match(Op1, m_AllOnes())) - return new ICmpInst(Pred, X, ConstantInt::getAllOnesValue(X->getType())); - } - - // Zero-equality checks are preserved through unsigned floating-point casts: - // icmp eq (bitcast (uitofp X)), 0 --> icmp eq X, 0 - // icmp ne (bitcast (uitofp X)), 0 --> icmp ne X, 0 - if (match(BCSrcOp, m_UIToFP(m_Value(X)))) - if (Cmp.isEquality() && match(Op1, m_Zero())) - return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType())); + + // Make sure the bitcast doesn't change the number of vector elements. + if (Bitcast->getSrcTy()->getScalarSizeInBits() == + Bitcast->getDestTy()->getScalarSizeInBits()) { + // Zero-equality and sign-bit checks are preserved through sitofp + bitcast. + Value *X; + if (match(BCSrcOp, m_SIToFP(m_Value(X)))) { + // icmp eq (bitcast (sitofp X)), 0 --> icmp eq X, 0 + // icmp ne (bitcast (sitofp X)), 0 --> icmp ne X, 0 + // icmp slt (bitcast (sitofp X)), 0 --> icmp slt X, 0 + // icmp sgt (bitcast (sitofp X)), 0 --> icmp sgt X, 0 + if ((Pred == ICmpInst::ICMP_EQ || Pred == ICmpInst::ICMP_SLT || + Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_SGT) && + match(Op1, m_Zero())) + return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType())); + + // icmp slt (bitcast (sitofp X)), 1 --> icmp slt X, 1 + if (Pred == ICmpInst::ICMP_SLT && match(Op1, m_One())) + return new ICmpInst(Pred, X, ConstantInt::get(X->getType(), 1)); + + // icmp sgt (bitcast (sitofp X)), -1 --> icmp sgt X, -1 + if (Pred == ICmpInst::ICMP_SGT && match(Op1, m_AllOnes())) + return new ICmpInst(Pred, X, + ConstantInt::getAllOnesValue(X->getType())); + } + + // Zero-equality checks are preserved through unsigned floating-point casts: + // icmp eq (bitcast (uitofp X)), 0 --> icmp eq X, 0 + // icmp ne (bitcast (uitofp X)), 0 --> icmp ne X, 0 + if (match(BCSrcOp, m_UIToFP(m_Value(X)))) + if (Cmp.isEquality() && match(Op1, m_Zero())) + return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType())); + } // Test to see if the operands of the icmp are casted versions of other // values. If the ptr->ptr cast can be stripped off both arguments, do so. diff --git a/test/Transforms/InstCombine/cast-int-icmp-eq-0.ll b/test/Transforms/InstCombine/cast-int-icmp-eq-0.ll index 9576c5c20ba..f18bfe7531c 100644 --- a/test/Transforms/InstCombine/cast-int-icmp-eq-0.ll +++ b/test/Transforms/InstCombine/cast-int-icmp-eq-0.ll @@ -651,3 +651,59 @@ define <3 x i1> @i16_cast_cmp_sgt_int_m1_sitofp_float_vec_undef(<3 x i16> %i) { ret <3 x i1> %cmp } +; Verify that the various forms of this transform are not applied when the +; bitcast changes the number of vector elements: +; icmp (bitcast ([su]itofp X)), Y -> icmp X, Y + +define <6 x i1> @i16_cast_cmp_sgt_int_m1_bitcast_vector_num_elements_sitofp(<3 x i16> %i) { +; CHECK-LABEL: @i16_cast_cmp_sgt_int_m1_bitcast_vector_num_elements_sitofp( +; CHECK-NEXT: [[F:%.*]] = sitofp <3 x i16> [[I:%.*]] to <3 x float> +; CHECK-NEXT: [[B:%.*]] = bitcast <3 x float> [[F]] to <6 x i16> +; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <6 x i16> [[B]], +; CHECK-NEXT: ret <6 x i1> [[CMP]] +; + %f = sitofp <3 x i16> %i to <3 x float> + %b = bitcast <3 x float> %f to <6 x i16> + %cmp = icmp sgt <6 x i16> %b, + ret <6 x i1> %cmp +} + +define i1 @i16_cast_cmp_sgt_int_m1_bitcast_vector_to_scalar_sitofp(<3 x i16> %i) { +; CHECK-LABEL: @i16_cast_cmp_sgt_int_m1_bitcast_vector_to_scalar_sitofp( +; CHECK-NEXT: [[F:%.*]] = sitofp <3 x i16> [[I:%.*]] to <3 x float> +; CHECK-NEXT: [[B:%.*]] = bitcast <3 x float> [[F]] to i96 +; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i96 [[B]], -1 +; CHECK-NEXT: ret i1 [[CMP]] +; + %f = sitofp <3 x i16> %i to <3 x float> + %b = bitcast <3 x float> %f to i96 + %cmp = icmp sgt i96 %b, -1 + ret i1 %cmp +} + + +define <6 x i1> @i16_cast_cmp_eq_int_0_bitcast_vector_num_elements_uitofp(<3 x i16> %i) { +; CHECK-LABEL: @i16_cast_cmp_eq_int_0_bitcast_vector_num_elements_uitofp( +; CHECK-NEXT: [[F:%.*]] = uitofp <3 x i16> [[I:%.*]] to <3 x float> +; CHECK-NEXT: [[B:%.*]] = bitcast <3 x float> [[F]] to <6 x i16> +; CHECK-NEXT: [[CMP:%.*]] = icmp eq <6 x i16> [[B]], zeroinitializer +; CHECK-NEXT: ret <6 x i1> [[CMP]] +; + %f = uitofp <3 x i16> %i to <3 x float> + %b = bitcast <3 x float> %f to <6 x i16> + %cmp = icmp eq <6 x i16> %b, + ret <6 x i1> %cmp +} + +define i1 @i16_cast_cmp_eq_int_0_bitcast_vector_to_scalar_uitofp(<3 x i16> %i) { +; CHECK-LABEL: @i16_cast_cmp_eq_int_0_bitcast_vector_to_scalar_uitofp( +; CHECK-NEXT: [[F:%.*]] = uitofp <3 x i16> [[I:%.*]] to <3 x float> +; CHECK-NEXT: [[B:%.*]] = bitcast <3 x float> [[F]] to i96 +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i96 [[B]], 0 +; CHECK-NEXT: ret i1 [[CMP]] +; + %f = uitofp <3 x i16> %i to <3 x float> + %b = bitcast <3 x float> %f to i96 + %cmp = icmp eq i96 %b, 0 + ret i1 %cmp +} -- 2.50.1