From 3fe1ea1aaeae16832f10c3721e36c893286d25ef Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 27 Aug 2018 22:41:44 +0000 Subject: [PATCH] [InstCombine] allow shuffle+binop canonicalization with widening shuffles This lines up with the behavior of an existing transform where if both operands of the binop are shuffled, we allow moving the binop before the shuffle regardless of whether the shuffle changes the size of the vector. llvm-svn: 340787 --- .../InstCombine/InstructionCombining.cpp | 18 ++++++++++++++---- test/Transforms/InstCombine/vec_shuffle.ll | 16 ++++++++-------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp index 99d3d478aaf..99874b31912 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1393,7 +1393,10 @@ Instruction *InstCombiner::foldShuffledBinop(BinaryOperator &Inst) { if (match(&Inst, m_c_BinOp( m_OneUse(m_ShuffleVector(m_Value(V1), m_Undef(), m_Constant(Mask))), m_Constant(C))) && - V1->getType() == Inst.getType()) { + V1->getType()->getVectorNumElements() <= VWidth) { + assert(Inst.getType()->getScalarType() == V1->getType()->getScalarType() && + "Shuffle should not change scalar type"); + unsigned V1Width = V1->getType()->getVectorNumElements(); // Find constant NewC that has property: // shuffle(NewC, ShMask) = C // If such constant does not exist (example: ShMask=<0,0> and C=<1,2>) @@ -1402,14 +1405,21 @@ Instruction *InstCombiner::foldShuffledBinop(BinaryOperator &Inst) { SmallVector ShMask; ShuffleVectorInst::getShuffleMask(Mask, ShMask); SmallVector - NewVecC(VWidth, UndefValue::get(C->getType()->getScalarType())); + NewVecC(V1Width, UndefValue::get(C->getType()->getScalarType())); bool MayChange = true; for (unsigned I = 0; I < VWidth; ++I) { if (ShMask[I] >= 0) { - assert(ShMask[I] < (int)VWidth); + assert(ShMask[I] < (int)VWidth && "Not expecting narrowing shuffle"); Constant *CElt = C->getAggregateElement(I); Constant *NewCElt = NewVecC[ShMask[I]]; - if (!CElt || (!isa(NewCElt) && NewCElt != CElt)) { + // Bail out if: + // 1. The constant vector contains a constant expression. + // 2. The shuffle needs an element of the constant vector that can't + // be mapped to a new constant vector. + // 3. This is a widening shuffle that copies elements of V1 into the + // extended elements (extending with undef is allowed). + if (!CElt || (!isa(NewCElt) && NewCElt != CElt) || + I >= V1Width) { MayChange = false; break; } diff --git a/test/Transforms/InstCombine/vec_shuffle.ll b/test/Transforms/InstCombine/vec_shuffle.ll index 17aad9325bb..cf09427557e 100644 --- a/test/Transforms/InstCombine/vec_shuffle.ll +++ b/test/Transforms/InstCombine/vec_shuffle.ll @@ -474,12 +474,12 @@ define <2 x float> @fmul_const_invalid_constant(<2 x float> %v) { ret <2 x float> %r } -; TODO: Reduce the width of the binop by moving it ahead of a shuffle. +; Reduce the width of the binop by moving it ahead of a shuffle. define <4 x i8> @widening_shuffle_add_1(<2 x i8> %x) { ; CHECK-LABEL: @widening_shuffle_add_1( -; CHECK-NEXT: [[WIDEX:%.*]] = shufflevector <2 x i8> [[X:%.*]], <2 x i8> undef, <4 x i32> -; CHECK-NEXT: [[R:%.*]] = add <4 x i8> [[WIDEX]], +; CHECK-NEXT: [[TMP1:%.*]] = add <2 x i8> [[X:%.*]], +; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i8> [[TMP1]], <2 x i8> undef, <4 x i32> ; CHECK-NEXT: ret <4 x i8> [[R]] ; %widex = shufflevector <2 x i8> %x, <2 x i8> undef, <4 x i32> @@ -487,12 +487,12 @@ define <4 x i8> @widening_shuffle_add_1(<2 x i8> %x) { ret <4 x i8> %r } -; TODO: Reduce the width of the binop by moving it ahead of a shuffle. +; Reduce the width of the binop by moving it ahead of a shuffle. define <4 x i8> @widening_shuffle_add_2(<2 x i8> %x) { ; CHECK-LABEL: @widening_shuffle_add_2( -; CHECK-NEXT: [[WIDEX:%.*]] = shufflevector <2 x i8> [[X:%.*]], <2 x i8> undef, <4 x i32> -; CHECK-NEXT: [[R:%.*]] = add <4 x i8> [[WIDEX]], +; CHECK-NEXT: [[TMP1:%.*]] = add <2 x i8> [[X:%.*]], +; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i8> [[TMP1]], <2 x i8> undef, <4 x i32> ; CHECK-NEXT: ret <4 x i8> [[R]] ; %widex = shufflevector <2 x i8> %x, <2 x i8> undef, <4 x i32> @@ -500,7 +500,7 @@ define <4 x i8> @widening_shuffle_add_2(<2 x i8> %x) { ret <4 x i8> %r } -; Widening shuffles have the same mask/constant constraint as non-size-changing shuffles. +; Negative test - widening shuffles have the same mask/constant constraint as non-size-changing shuffles. define <4 x i8> @widening_shuffle_add_invalid_constant(<2 x i8> %x) { ; CHECK-LABEL: @widening_shuffle_add_invalid_constant( @@ -513,7 +513,7 @@ define <4 x i8> @widening_shuffle_add_invalid_constant(<2 x i8> %x) { ret <4 x i8> %r } -; And widening shuffles have an additional constraint - they must not extend with anything but undefs. +; Negative test - widening shuffles have an additional constraint: they must not extend with anything but undefs. define <4 x i8> @widening_shuffle_add_invalid_mask(<2 x i8> %x) { ; CHECK-LABEL: @widening_shuffle_add_invalid_mask(