instcombine transforms vector loads that are only used by

extractelement operations into a bitcast of the pointer,
then a gep, then a scalar load.  Disable this when the vector
only has one element, because it leads to infinite loops in
instcombine (PR4908).

This transformation seems like a really bad idea to me, as it
will likely disable CSE of vector load/stores etc and can be
better done in the code generator when profitable.  This
goes all the way back to the first days of packed types,
r25299 specifically.

I'll let those people who care about the performance of vector
code decide what to do with this.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@81185 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp
index 962a545d640d12a79e3c10bb7859091bde71d256..17d75555c45032dd767d51b01d10680839157daa 100644 (file)
--- a/lib/Transforms/Scalar/InstructionCombining.cpp
+++ b/lib/Transforms/Scalar/InstructionCombining.cpp
@@ -12080,7 +12080,7 @@ Instruction *InstCombiner::visitExtractElementInst(ExtractElementInst &EI) {
     // that element. When the elements are not identical, we cannot replace yet
     // (we do that below, but only when the index is constant).
     Constant *op0 = C->getOperand(0);
-    for (unsigned i = 1; i < C->getNumOperands(); ++i)
+    for (unsigned i = 1; i != C->getNumOperands(); ++i)
       if (C->getOperand(i) != op0) {
         op0 = 0; 
         break;
@@ -12093,8 +12093,7 @@ Instruction *InstCombiner::visitExtractElementInst(ExtractElementInst &EI) {
   // find a previously computed scalar that was inserted into the vector.
   if (ConstantInt *IdxC = dyn_cast<ConstantInt>(EI.getOperand(1))) {
     unsigned IndexVal = IdxC->getZExtValue();
-    unsigned VectorWidth = 
-      cast<VectorType>(EI.getOperand(0)->getType())->getNumElements();
+    unsigned VectorWidth = EI.getVectorOperandType()->getNumElements();
       
     // If this is extracting an invalid index, turn this into undef, to avoid
     // crashing the code below.
@@ -12146,25 +12145,31 @@ Instruction *InstCombiner::visitExtractElementInst(ExtractElementInst &EI) {
           return BinaryOperator::Create(BO->getOpcode(), newEI0, newEI1);
         }
       } else if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
-        unsigned AS = LI->getPointerAddressSpace();
-        Value *Ptr = Builder->CreateBitCast(I->getOperand(0),
-                                            PointerType::get(EI.getType(), AS),
-                                            I->getOperand(0)->getName());
-        Value *GEP =
-          Builder->CreateInBoundsGEP(Ptr, EI.getOperand(1),
-                                     I->getName()+".gep");
-        
-        LoadInst *Load = Builder->CreateLoad(GEP, "tmp");
-
-        // Make sure the Load goes before the load instruction in the source,
-        // not wherever the extract happens to be.
-        if (Instruction *P = dyn_cast<Instruction>(Ptr))
-          P->moveBefore(I);
-        if (Instruction *G = dyn_cast<Instruction>(GEP))
-          G->moveBefore(I);
-        Load->moveBefore(I);
-        
-        return ReplaceInstUsesWith(EI, Load);
+//        r25299
+        // Instead of loading a vector, then doing an extract element out of it,
+        // just bitcast the pointer operand, do a gep, then load the result.
+        // This shrinks the vector load to a scalar load.
+        if (EI.getVectorOperandType()->getNumElements() != 1) {
+          unsigned AS = LI->getPointerAddressSpace();
+          Value *Ptr = Builder->CreateBitCast(I->getOperand(0),
+                                              PointerType::get(EI.getType(), AS),
+                                              I->getOperand(0)->getName());
+          Value *GEP =
+            Builder->CreateInBoundsGEP(Ptr, EI.getOperand(1),
+                                       I->getName()+".gep");
+          
+          LoadInst *Load = Builder->CreateLoad(GEP, "tmp");
+
+          // Make sure the Load goes before the load instruction in the source,
+          // not wherever the extract happens to be.
+          if (Instruction *P = dyn_cast<Instruction>(Ptr))
+            P->moveBefore(I);
+          if (Instruction *G = dyn_cast<Instruction>(GEP))
+            G->moveBefore(I);
+          Load->moveBefore(I);
+          
+          return ReplaceInstUsesWith(EI, Load);
+        }
       }
     }
     if (InsertElementInst *IE = dyn_cast<InsertElementInst>(I)) {

diff --git a/test/Transforms/InstCombine/crash.ll b/test/Transforms/InstCombine/crash.ll
index c252ac9dccf8c72ce09ae0bd7639f8740091afe5..d475ab5bc57cf44642a0be12dc1ec2bed3743b3c 100644 (file)
--- a/test/Transforms/InstCombine/crash.ll
+++ b/test/Transforms/InstCombine/crash.ll
@@ -31,3 +31,16 @@ entry:
   ret <2 x i64> %conv3.i44
 }
 
+
+; PR4908
+define void @test2(<1 x i16>* nocapture %b, i32* nocapture %c) nounwind ssp {
+entry:
+  %arrayidx = getelementptr inbounds <1 x i16>* %b, i64 undef ; <<1 x i16>*>
+  %tmp2 = load <1 x i16>* %arrayidx               ; <<1 x i16>> [#uses=1]
+  %tmp6 = bitcast <1 x i16> %tmp2 to i16          ; <i16> [#uses=1]
+  %tmp7 = zext i16 %tmp6 to i32                   ; <i32> [#uses=1]
+  %ins = or i32 0, %tmp7                          ; <i32> [#uses=1]
+  %arrayidx20 = getelementptr inbounds i32* %c, i64 undef ; <i32*> [#uses=1]
+  store i32 %ins, i32* %arrayidx20
+  ret void
+}