From 8c655150827b5d56772e628994db08441c554097 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 3 Dec 2018 21:15:17 +0000 Subject: [PATCH] [InstCombine] fix undef propagation bug with shuffle+binop When we have a shuffle that extends a source vector with undefs and then do some binop on that, we must make sure that the extra elements remain undef with that binop if we reverse the order of the binop and shuffle. 'or' is probably the easiest example to show the bug because 'or C, undef --> -1' (not undef). But there are other opcode/constant combinations where this is true as shown by the 'shl' test. llvm-svn: 348191 --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 16 ++++++++++++++++ llvm/test/Transforms/InstCombine/vec_shuffle.ll | 12 ++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 13903d1..7c9dbcb6 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1455,6 +1455,22 @@ Instruction *InstCombiner::foldVectorBinop(BinaryOperator &Inst) { } NewVecC[ShMask[I]] = CElt; } + // If this is a widening shuffle, we must be able to extend with undef + // elements. If the original binop does not produce an undef in the high + // lanes, then this transform is not safe. + // TODO: We could shuffle those non-undef constant values into the + // result by using a constant vector (rather than an undef vector) + // as operand 1 of the new binop, but that might be too aggressive + // for target-independent shuffle creation. + if (I >= SrcVecNumElts) { + Constant *MaybeUndef = + ConstOp1 ? ConstantExpr::get(Opcode, UndefScalar, CElt) + : ConstantExpr::get(Opcode, CElt, UndefScalar); + if (!isa(MaybeUndef)) { + MayChange = false; + break; + } + } } if (MayChange) { Constant *NewC = ConstantVector::get(NewVecC); diff --git a/llvm/test/Transforms/InstCombine/vec_shuffle.ll b/llvm/test/Transforms/InstCombine/vec_shuffle.ll index 6e50045..b82c811 100644 --- a/llvm/test/Transforms/InstCombine/vec_shuffle.ll +++ b/llvm/test/Transforms/InstCombine/vec_shuffle.ll @@ -633,13 +633,13 @@ define <4 x i16> @widening_shuffle_shl_constant_op1(<2 x i16> %v) { ret <4 x i16> %bo } -; FIXME: A binop that does not produce undef in the high lanes can not be moved before the shuffle. +; A binop that does not produce undef in the high lanes can not be moved before the shuffle. ; This is not ok because 'shl undef, 1 (or 2)' --> 0' but moving the shuffle results in undef instead. define <4 x i16> @widening_shuffle_shl_constant_op1_non0(<2 x i16> %v) { ; CHECK-LABEL: @widening_shuffle_shl_constant_op1_non0( -; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i16> [[V:%.*]], -; CHECK-NEXT: [[BO:%.*]] = shufflevector <2 x i16> [[TMP1]], <2 x i16> undef, <4 x i32> +; CHECK-NEXT: [[SHUF:%.*]] = shufflevector <2 x i16> [[V:%.*]], <2 x i16> undef, <4 x i32> +; CHECK-NEXT: [[BO:%.*]] = shl <4 x i16> [[SHUF]], ; CHECK-NEXT: ret <4 x i16> [[BO]] ; %shuf = shufflevector <2 x i16> %v, <2 x i16> undef, <4 x i32> @@ -647,13 +647,13 @@ define <4 x i16> @widening_shuffle_shl_constant_op1_non0(<2 x i16> %v) { ret <4 x i16> %bo } -; FIXME: A binop that does not produce undef in the high lanes can not be moved before the shuffle. +; A binop that does not produce undef in the high lanes can not be moved before the shuffle. ; This is not ok because 'or -1, undef --> -1' but moving the shuffle results in undef instead. define <4 x i16> @widening_shuffle_or(<2 x i16> %v) { ; CHECK-LABEL: @widening_shuffle_or( -; CHECK-NEXT: [[TMP1:%.*]] = or <2 x i16> [[V:%.*]], -; CHECK-NEXT: [[BO:%.*]] = shufflevector <2 x i16> [[TMP1]], <2 x i16> undef, <4 x i32> +; CHECK-NEXT: [[SHUF:%.*]] = shufflevector <2 x i16> [[V:%.*]], <2 x i16> undef, <4 x i32> +; CHECK-NEXT: [[BO:%.*]] = or <4 x i16> [[SHUF]], ; CHECK-NEXT: ret <4 x i16> [[BO]] ; %shuf = shufflevector <2 x i16> %v, <2 x i16> undef, <4 x i32> -- 2.7.4