[llvm] r343329 - [InstCombine] don't propagate wider shufflevector arguments to predecessors

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Fri Sep 28 08:24:41 2018
New Revision: 343329

URL: http://llvm.org/viewvc/llvm-project?rev=343329&view=rev
Log:
[InstCombine] don't propagate wider shufflevector arguments to predecessors

InstCombine would propagate shufflevector insts that had wider output vectors onto 
predecessors, which would sometimes push undef's onto the divisor of a div/rem and 
result in bad codegen.

I've fixed this by just banning propagating shufflevector back if the result of 
the shufflevector is wider than the input vectors.

Patch by: @sheredom (Neil Henning)

Differential Revision: https://reviews.llvm.org/D52548


Modified:
    llvm/trunk/include/llvm/IR/Instructions.h
    llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
    llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll

Modified: llvm/trunk/include/llvm/IR/Instructions.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instructions.h?rev=343329&r1=343328&r2=343329&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Instructions.h (original)
+++ llvm/trunk/include/llvm/IR/Instructions.h Fri Sep 28 08:24:41 2018
@@ -2457,13 +2457,23 @@ public:
 
   /// Return true if this shuffle returns a vector with a different number of
   /// elements than its source vectors.
-  /// Example: shufflevector <4 x n> A, <4 x n> B, <1,2>
+  /// Examples: shufflevector <4 x n> A, <4 x n> B, <1,2,3>
+  ///           shufflevector <4 x n> A, <4 x n> B, <1,2,3,4,5>
   bool changesLength() const {
     unsigned NumSourceElts = Op<0>()->getType()->getVectorNumElements();
     unsigned NumMaskElts = getMask()->getType()->getVectorNumElements();
     return NumSourceElts != NumMaskElts;
   }
 
+  /// Return true if this shuffle returns a vector with a greater number of
+  /// elements than its source vectors.
+  /// Example: shufflevector <2 x n> A, <2 x n> B, <1,2,3>
+  bool increasesLength() const {
+    unsigned NumSourceElts = Op<0>()->getType()->getVectorNumElements();
+    unsigned NumMaskElts = getMask()->getType()->getVectorNumElements();
+    return NumSourceElts < NumMaskElts;
+  }
+
   /// Return true if this shuffle mask chooses elements from exactly one source
   /// vector.
   /// Example: <7,5,undef,7>

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=343329&r1=343328&r2=343329&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp Fri Sep 28 08:24:41 2018
@@ -1464,7 +1464,8 @@ Instruction *InstCombiner::visitShuffleV
     if (isRHSID) return replaceInstUsesWith(SVI, RHS);
   }
 
-  if (isa<UndefValue>(RHS) && CanEvaluateShuffled(LHS, Mask)) {
+  if (isa<UndefValue>(RHS) && !SVI.increasesLength() &&
+      CanEvaluateShuffled(LHS, Mask)) {
     Value *V = EvaluateInDifferentElementOrder(LHS, Mask);
     return replaceInstUsesWith(SVI, V);
   }

Modified: llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll?rev=343329&r1=343328&r2=343329&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll Fri Sep 28 08:24:41 2018
@@ -184,14 +184,15 @@ define <2 x i8> @test13a(i8 %x1, i8 %x2)
   ret <2 x i8> %D
 }
 
-; TODO: Increasing length of vector ops is not a good canonicalization.
- 
+; Increasing length of vector ops is not a good canonicalization.
+
 define <3 x i32> @add_wider(i32 %y, i32 %z) {
-; CHECK-LABEL: @add(
-; CHECK-NEXT:    [[TMP1:%.*]] = insertelement <3 x i32> undef, i32 [[Y:%.*]], i32 0
-; CHECK-NEXT:    [[TMP2:%.*]] = insertelement <3 x i32> [[TMP1]], i32 [[Z:%.*]], i32 1
-; CHECK-NEXT:    [[TMP3:%.*]] = add <3 x i32> [[TMP2]], <i32 255, i32 255, i32 undef>
-; CHECK-NEXT:    ret <3 x i32> [[TMP3]]
+; CHECK-LABEL: @add_wider(
+; CHECK-NEXT:    [[I0:%.*]] = insertelement <2 x i32> undef, i32 [[Y:%.*]], i32 0
+; CHECK-NEXT:    [[I1:%.*]] = insertelement <2 x i32> [[I0]], i32 [[Z:%.*]], i32 1
+; CHECK-NEXT:    [[A:%.*]] = add <2 x i32> [[I1]], <i32 255, i32 255>
+; CHECK-NEXT:    [[EXT:%.*]] = shufflevector <2 x i32> [[A]], <2 x i32> undef, <3 x i32> <i32 0, i32 1, i32 undef>
+; CHECK-NEXT:    ret <3 x i32> [[EXT]]
 ;
   %i0 = insertelement <2 x i32> undef, i32 %y, i32 0
   %i1 = insertelement <2 x i32> %i0, i32 %z, i32 1
@@ -200,11 +201,15 @@ define <3 x i32> @add_wider(i32 %y, i32
   ret <3 x i32> %ext
 }
 
-; FIXME: Increasing length of vector ops must be safe from illegal undef propagation.
+; Increasing length of vector ops must be safe from illegal undef propagation.
 
 define <3 x i32> @div_wider(i32 %y, i32 %z) {
-; CHECK-LABEL: @div(
-; CHECK-NEXT:    ret <3 x i32> undef
+; CHECK-LABEL: @div_wider(
+; CHECK-NEXT:    [[I0:%.*]] = insertelement <2 x i32> undef, i32 [[Y:%.*]], i32 0
+; CHECK-NEXT:    [[I1:%.*]] = insertelement <2 x i32> [[I0]], i32 [[Z:%.*]], i32 1
+; CHECK-NEXT:    [[A:%.*]] = sdiv <2 x i32> [[I1]], <i32 255, i32 255>
+; CHECK-NEXT:    [[EXT:%.*]] = shufflevector <2 x i32> [[A]], <2 x i32> undef, <3 x i32> <i32 0, i32 1, i32 undef>
+; CHECK-NEXT:    ret <3 x i32> [[EXT]]
 ;
   %i0 = insertelement <2 x i32> undef, i32 %y, i32 0
   %i1 = insertelement <2 x i32> %i0, i32 %z, i32 1