[llvm] r336558 - [InstCombine] generalize safe vector constant utility

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Mon Jul  9 09:16:51 2018
New Revision: 336558

URL: http://llvm.org/viewvc/llvm-project?rev=336558&view=rev
Log:
[InstCombine] generalize safe vector constant utility

This is almost NFC, but there could be some case where the original
code had undefs in the constants (rather than just the shuffle mask),
and we'll use safe constants rather than undefs now.

The FIXME noted in foldShuffledBinop() is already visible in existing
tests, so correcting that is the next step.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
    llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h?rev=336558&r1=336557&r2=336558&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h Mon Jul  9 09:16:51 2018
@@ -212,19 +212,26 @@ IntrinsicIDToOverflowCheckFlavor(unsigne
   }
 }
 
-/// Integer division/remainder require special handling to avoid undefined
+/// Some binary operators require special handling to avoid poison and undefined
 /// behavior. If a constant vector has undef elements, replace those undefs with
-/// '1' because that's always safe to execute.
-static inline Constant *getSafeVectorConstantForIntDivRem(Constant *In) {
-  assert(In->getType()->isVectorTy() && "Not expecting scalars here");
-  assert(In->getType()->getVectorElementType()->isIntegerTy() &&
-         "Not expecting FP opcodes/operands/constants here");
+/// identity constants because those are always safe to execute. If no identity
+/// constant exists, replace undef with '1' or '1.0'.
+static inline Constant *getSafeVectorConstantForBinop(
+      BinaryOperator::BinaryOps Opcode, Constant *In) {
+  Type *Ty = In->getType();
+  assert(Ty->isVectorTy() && "Not expecting scalars here");
 
-  unsigned NumElts = In->getType()->getVectorNumElements();
+  Type *EltTy = Ty->getVectorElementType();
+  Constant *IdentityC = ConstantExpr::getBinOpIdentity(Opcode, EltTy, true);
+  if (!IdentityC)
+    IdentityC = EltTy->isIntegerTy() ? ConstantInt::get(EltTy, 1):
+                                       ConstantFP::get(EltTy, 1.0);
+
+  unsigned NumElts = Ty->getVectorNumElements();
   SmallVector<Constant *, 16> Out(NumElts);
   for (unsigned i = 0; i != NumElts; ++i) {
     Constant *C = In->getAggregateElement(i);
-    Out[i] = isa<UndefValue>(C) ? ConstantInt::get(C->getType(), 1) : C;
+    Out[i] = isa<UndefValue>(C) ? IdentityC : C;
   }
   return ConstantVector::get(Out);
 }

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=336558&r1=336557&r2=336558&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp Mon Jul  9 09:16:51 2018
@@ -1288,33 +1288,18 @@ static Instruction *foldSelectShuffle(Sh
       Mask->containsUndefElement() &&
       (Instruction::isIntDivRem(BOpc) || Instruction::isShift(BOpc));
 
-  Constant *NewC;
+  // Select the constant elements needed for the single binop.
+  Constant *NewC = ConstantExpr::getShuffleVector(C0, C1, Mask);
   Value *V;
   if (X == Y) {
-    NewC = ConstantExpr::getShuffleVector(C0, C1, Mask);
-
     // The new binop constant must not have any potential for extra poison/UB.
     if (MightCreatePoisonOrUB) {
       // TODO: Use getBinOpAbsorber for LHS replacement constants?
       if (!ConstantsAreOp1)
         return nullptr;
 
-      Type *EltTy = Shuf.getType()->getVectorElementType();
-      auto *IdC = ConstantExpr::getBinOpIdentity(BOpc, EltTy, true);
-      if (!IdC)
-        return nullptr;
-
-      // Replace undef elements caused by the mask with identity constants.
-      NewC = ConstantExpr::getShuffleVector(C0, C1, Mask);
-      unsigned NumElts = Shuf.getType()->getVectorNumElements();
-      SmallVector<Constant *, 16> VectorOfNewC(NumElts);
-      for (unsigned i = 0; i != NumElts; i++) {
-        if (isa<UndefValue>(Mask->getAggregateElement(i)))
-          VectorOfNewC[i] = IdC;
-        else
-          VectorOfNewC[i] = NewC->getAggregateElement(i);
-      }
-      NewC = ConstantVector::get(VectorOfNewC);
+      // Replace undef elements with identity constants.
+      NewC = getSafeVectorConstantForBinop(BOpc, NewC);
     }
 
     // Remove a binop and the shuffle by rearranging the constant:
@@ -1340,7 +1325,6 @@ static Instruction *foldSelectShuffle(Sh
     // Select the variable vectors first, then perform the binop:
     // shuffle (op X, C0), (op Y, C1), M --> op (shuffle X, Y, M), C'
     // shuffle (op C0, X), (op C1, Y), M --> op C', (shuffle X, Y, M)
-    NewC = ConstantExpr::getShuffleVector(C0, C1, Mask);
     V = Builder.CreateShuffleVector(X, Y, Mask);
   }
 

Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=336558&r1=336557&r2=336558&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Mon Jul  9 09:16:51 2018
@@ -1423,8 +1423,10 @@ Instruction *InstCombiner::foldShuffledB
       // All other binop opcodes are always safe to speculate, and therefore, it
       // is fine to include undef elements for unused lanes (and using undefs
       // may help optimization).
+      // FIXME: This transform is also not poison-safe. Eg, shift-by-undef would
+      // create poison that may not exist in the original code.
       if (Inst.isIntDivRem())
-        NewC = getSafeVectorConstantForIntDivRem(NewC);
+        NewC = getSafeVectorConstantForBinop(Inst.getOpcode(), NewC);
       
       // Op(shuffle(V1, Mask), C) -> shuffle(Op(V1, NewC), Mask)
       // Op(C, shuffle(V1, Mask)) -> shuffle(Op(NewC, V1), Mask)