Extended LoadStoreVectorizer to vectorize subchains.

[asbirlea]

Summary

LSV used to abort vectorizing a chain for interleaved load/store accesses that alias.
Allow a valid prefix of the chain to be vectorized, mark just the prefix and retry vectorizing the remaining chain.

[jlebar]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

113 ↗

(On Diff #63158)

Please update comment.

114 ↗

(On Diff #63158)

This probably also deserves a new name.

453 ↗

(On Diff #63158)

Suppose we have

• load
• load
• fn_with_side_effects
• load

Don't we want to (try to) vectorize the first two loads? Or, do you want to handle that in a separate patch?

460 ↗

(On Diff #63158)

Nit, I'm not wild about reusing "Idx" for something different. (I mean, it's still an index, but over a completely different list.)

675 ↗

(On Diff #63158)

Calling this param "FullChain" is weird, since when we call this recursively, we pass what I can only call a partial chain. :)

701 ↗

(On Diff #63158)

Is "VectorizedSet" the right name? In this case we're saying that we didn't vectorize anything (and we're returning false), yet we're adding these instructions to the set...

704 ↗

(On Diff #63158)

If this is just describing the call to getBoundaryInstrs, I don't think it needs a comment. If it's describing something else, can we reword it? I don't think "enclosing" is the right word. Maybe "Get the first and last instructions in FullChain," but that's basically exactly what getBoundaryInstrs' comment says.

707 ↗

(On Diff #63158)

Nit, end sentences with periods.

714 ↗

(On Diff #63158)

This indentation is very confusing. I think it would be OK to put the comment inside the "else"? In which case I'd suggest also putting braces (although some folks around here would leave them out, which really mystifies me).

716 ↗

(On Diff #63158)

VectorizedSet->push_back(FullChain.front());

718 ↗

(On Diff #63158)

I think this would be clearer as two separate if statements (repeating the "return false"). I'd rather repeat one short line than add an extra level of nesting.

if (StopChain == 0) {
  // No vectorization possible, bail out.
  VectorizedSet->append(...);
  return false;
}
if (StopChain == 1) {
  // Failed after the first instruction.  Discard it and try the smaller chain.
  VectorizedSet->push_back(FullChain.front());
  return false;
}

761 ↗

(On Diff #63158)

"discarded for future vectorization" is awkward. Maybe "We won't try again to vectorize the elements of chain, regardless of whether we succeed below."

[asbirlea]

Address comments.

[jlebar]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

443 ↗

(On Diff #63359)

Did you intend to address this comment in your latest update?

[asbirlea]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

443 ↗

(On Diff #63359)

No, let's handle that in a separate patch. Yes, it's another feature that should be added.

[jlebar]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

112 ↗

(On Diff #63359)

The first sentence in this comment no longer really applies. Can we make this more cohesive?

113 ↗

(On Diff #63359)

Per IRL, maybe vectorizablePrefixEnd or vectorizablePrefixEndIdx would be a better name?

357 ↗

(On Diff #63359)

Hm, why is this diff showing up here? Shouldn't this be part of D22071?

450 ↗

(On Diff #63359)

Well, calling the one "Idx" and the other "Index" doesn't entirely solve the problem of using the same name for the two variables. Now they're different names to the computer, but still basically the same name to humans. :)

How about InstrIdx and ChainIdx? Then you could rename VIdx to InstrIdx, which would be a plus for readability, I think (since these *are* the same indices). (The problem with s/Idx/VIdx/ is that the first loop is really over instructions, not Values.)

624 ↗

(On Diff #63359)

Capital letters for variables. Although, why did we lose the range-based for loop in the first place? Was it just so we could do Head = -1? If so, a flag variable would be much more clear, I think.

658 ↗

(On Diff #63359)

We shouldn't need a separate variable for this. Can we just pass VectorizedValues (needs to be renamed) to vectorizeLoadChain directly (after changing the type accepted by that function)?

[asbirlea]

Update to latest.

[asbirlea]

Address comments.

[asbirlea]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

112 ↗

(On Diff #63605)

Well, it still applies. It's still checking for the existance of those instructions but using the info differently. I shortened the comment somewhat.

356 ↗

(On Diff #63605)

Because I accidentally did the updates on this one some time ago :). Then I redid them on the other one.

449–455 ↗

(On Diff #63605)

Also updated VV/V and VVIdx/VIdx. They still need different names from InstrIndx, since the same index is used for both Chain and Mem in the loop above, and now we need two.

623–632 ↗

(On Diff #63605)

For Heads it's fine to use the range-based loop (updated). For Tails I need the iterator to get Heads[TailsIterator].

[jlebar]

lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

435 ↗

(On Diff #63745)

Do we need to declare E? Can we just say I != To?

465 ↗

(On Diff #63745)

Please be sure to clang-format the lines you've touched in this patch (e.g. with git-clang-format HEAD^) before submitting.

112 ↗

(On Diff #63605)

I like it now. :)

Except we should be consistent *at least within a single comment* as far as whether we're using indicative ("checks") or imperative ("return").

623–632 ↗

(On Diff #63605)

OK. I would still very much prefer to use a separate flag variable rather than repurposing Head for that purpose, though.

[asbirlea]

Address comments.

llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

private:
 
   /// "Legalize" the vector type that would be produced by combining \p
   /// ElementSizeBits elements in \p Chain. Break into two pieces such that the
   /// total size of each piece is 1, 2 or a multiple of 4 bytes. \p Chain is
   /// expected to have more than 4 elements.
   std::pair<ArrayRef<Value *>, ArrayRef<Value *>>
   splitOddVectorElts(ArrayRef<Value *> Chain, unsigned ElementSizeBits);
 
-  /// Checks if there are any instructions which may affect the memory accessed
-  /// in the chain between \p From and \p To. The elements of \p Chain should be
-  /// all loads or all stores.
-  bool isVectorizable(ArrayRef<Value *> Chain, BasicBlock::iterator From,
+  /// Checks for instructions which may affect the memory accessed
+  /// in the chain between \p From and \p To. Returns Index, where
+  /// \p Chain[0, Index) is the largest vectorizable chain prefix.
+  /// The elements of \p Chain should be all loads or all stores.
+  unsigned getVectorizablePrefixEndIdx(ArrayRef<Value *> Chain,
+                                       BasicBlock::iterator From,
                                        BasicBlock::iterator To);
 
   /// Collects load and store instructions to vectorize.
   void collectInstructions(BasicBlock *BB);
 
   /// Processes the collected instructions, the \p Map. The elements of \p Map
   /// should be all loads or all stores.
   bool vectorizeChains(ValueListMap &Map);
 
   /// Finds the load/stores to consecutive memory addresses and vectorizes them.
   bool vectorizeInstructions(ArrayRef<Value *> Instrs);
 
   /// Vectorizes the load instructions in Chain.
-  bool vectorizeLoadChain(ArrayRef<Value *> Chain);
+  bool vectorizeLoadChain(ArrayRef<Value *> Chain,
+                          SmallPtrSet<Value *, 16> *InstructionsProcessed);
 
   /// Vectorizes the store instructions in Chain.
-  bool vectorizeStoreChain(ArrayRef<Value *> Chain);
+  bool vectorizeStoreChain(ArrayRef<Value *> Chain,
+                           SmallPtrSet<Value *, 16> *InstructionsProcessed);
 
   /// Check if this load/store access is misaligned accesses
   bool accessIsMisaligned(unsigned SzInBytes, unsigned AddressSpace,
                           unsigned Alignment);
 };
 
 class LoadStoreVectorizer : public FunctionPass {
 public:
Vectorizer::splitOddVectorElts(ArrayRef<Value *> Chain,
                                unsigned ElementSizeBits) {
   unsigned ElemSizeInBytes = ElementSizeBits / 8;
   unsigned SizeInBytes = ElemSizeInBytes * Chain.size();
   unsigned NumRight = (SizeInBytes % 4) / ElemSizeInBytes;
   unsigned NumLeft = Chain.size() - NumRight;
   return std::make_pair(Chain.slice(0, NumLeft), Chain.slice(NumLeft));
 }
 
-bool Vectorizer::isVectorizable(ArrayRef<Value *> Chain,
+unsigned Vectorizer::getVectorizablePrefixEndIdx(ArrayRef<Value *> Chain,
                                                  BasicBlock::iterator From,
                                                  BasicBlock::iterator To) {
   SmallVector<std::pair<Value *, unsigned>, 16> MemoryInstrs;
   SmallVector<std::pair<Value *, unsigned>, 16> ChainInstrs;
 
-  unsigned Idx = 0;
-  for (auto I = From, E = To; I != E; ++I, ++Idx) {
+  unsigned InstrIdx = 0;
+  for (auto I = From; I != To; ++I, ++InstrIdx) {
     if (isa<LoadInst>(I) || isa<StoreInst>(I)) {
       if (!is_contained(Chain, &*I))
-        MemoryInstrs.push_back({&*I, Idx});
+        MemoryInstrs.push_back({&*I, InstrIdx});
       else
-        ChainInstrs.push_back({&*I, Idx});
+        ChainInstrs.push_back({&*I, InstrIdx});
     } else if (I->mayHaveSideEffects()) {
       DEBUG(dbgs() << "LSV: Found side-effecting operation: " << *I << '\n');
-      return false;
+      return 0;
     }
   }
 
   assert(Chain.size() == ChainInstrs.size() &&
          "All instructions in the Chain must exist in [From, To).");
 
-  for (auto EntryMem : MemoryInstrs) {
-    Value *V = EntryMem.first;
-    unsigned VIdx = EntryMem.second;
-    for (auto EntryChain : ChainInstrs) {
-      Value *VV = EntryChain.first;
-      unsigned VVIdx = EntryChain.second;
-      if (isa<LoadInst>(V) && isa<LoadInst>(VV))
+  unsigned ChainIdx = 0;
+    Value *ChainInstrValue = EntryChain.first;
+    unsigned ChainInstrIdx = EntryChain.second;
+    for (auto EntryMem : MemoryInstrs) {
+      Value *MemInstrValue = EntryMem.first;
+      unsigned MemInstrIdx = EntryMem.second;
+      if (isa<LoadInst>(MemInstrValue) && isa<LoadInst>(ChainInstrValue))
         continue;
 
       // We can ignore the alias as long as the load comes before the store,
       // because that means we won't be moving the load past the store to
       // vectorize it (the vectorized load is inserted at the location of the
       // first load in the chain).
-      if (isa<StoreInst>(V) && isa<LoadInst>(VV) && VVIdx < VIdx)
+      if (isa<StoreInst>(MemInstrValue) && isa<LoadInst>(ChainInstrValue) &&
+          ChainInstrIdx < MemInstrIdx)
         continue;
 
       // Same case, but in reverse.
-      if (isa<LoadInst>(V) && isa<StoreInst>(VV) && VVIdx > VIdx)
+      if (isa<LoadInst>(MemInstrValue) && isa<StoreInst>(ChainInstrValue) &&
+          ChainInstrIdx > MemInstrIdx)
         continue;
 
-      Instruction *M0 = cast<Instruction>(V);
-      Instruction *M1 = cast<Instruction>(VV);
+      Instruction *M0 = cast<Instruction>(MemInstrValue);
+      Instruction *M1 = cast<Instruction>(ChainInstrValue);
 
       if (!AA.isNoAlias(MemoryLocation::get(M0), MemoryLocation::get(M1))) {
         DEBUG({
           Value *Ptr0 = getPointerOperand(M0);
           Value *Ptr1 = getPointerOperand(M1);
 
           dbgs() << "LSV: Found alias.\n"
                     "        Aliasing instruction and pointer:\n"
-                 << *V << " aliases " << *Ptr0 << '\n'
+                 << *MemInstrValue << " aliases " << *Ptr0 << '\n'
                  << "        Aliased instruction and pointer:\n"
-                 << *VV << " aliases " << *Ptr1 << '\n';
+                 << *ChainInstrValue << " aliases " << *Ptr1 << '\n';
         });
 
-        return false;
+        return ChainIdx;
       }
     }
+    ChainIdx++;
   }
-
-  return true;
+  return Chain.size();
 }
 
 void Vectorizer::collectInstructions(BasicBlock *BB) {
   LoadRefs.clear();
   StoreRefs.clear();
 
   for (Instruction &I : *BB) {
     if (!I.mayReadOrWriteMemory())
for (int j = e - 1; j >= 0; --j) {
         Tails.insert(j);
         Heads.insert(i);
         ConsecutiveChain[i] = j;
       }
     }
   }
 
   bool Changed = false;
-  SmallPtrSet<Value *, 16> VectorizedValues;
+  SmallPtrSet<Value *, 16> InstructionsProcessed;
 
   for (int Head : Heads) {
-    if (Tails.count(Head))
+    if (InstructionsProcessed.count(Instrs[Head]))
+      continue;
+    bool longerChainExists = false;
+    for (unsigned TIt = 0; TIt < Tails.size(); TIt++)
+      if (Head == Tails[TIt] &&
+          !InstructionsProcessed.count(Instrs[Heads[TIt]])) {
+        longerChainExists = true;
+        break;
+      }
+    if (longerChainExists)
       continue;
 
     // We found an instr that starts a chain. Now follow the chain and try to
     // vectorize it.
     SmallVector<Value *, 16> Operands;
     int I = Head;
     while (I != -1 && (Tails.count(I) || Heads.count(I))) {
-      if (VectorizedValues.count(Instrs[I]))
+      if (InstructionsProcessed.count(Instrs[I]))
         break;
 
       Operands.push_back(Instrs[I]);
       I = ConsecutiveChain[I];
     }
 
     bool Vectorized = false;
     if (isa<LoadInst>(*Operands.begin()))
-      Vectorized = vectorizeLoadChain(Operands);
+      Vectorized = vectorizeLoadChain(Operands, &InstructionsProcessed);
     else
-      Vectorized = vectorizeStoreChain(Operands);
+      Vectorized = vectorizeStoreChain(Operands, &InstructionsProcessed);
 
-    // Mark the vectorized instructions so that we don't vectorize them again.
-    if (Vectorized)
-      VectorizedValues.insert(Operands.begin(), Operands.end());
     Changed |= Vectorized;
   }
 
   return Changed;
 }
 
-bool Vectorizer::vectorizeStoreChain(ArrayRef<Value *> Chain) {
+bool Vectorizer::vectorizeStoreChain(
+    ArrayRef<Value *> Chain, SmallPtrSet<Value *, 16> *InstructionsProcessed) {
   StoreInst *S0 = cast<StoreInst>(Chain[0]);
 
   // If the vector has an int element, default to int for the whole load.
   Type *StoreTy;
   for (const auto &V : Chain) {
     StoreTy = cast<StoreInst>(V)->getValueOperand()->getType();
     if (StoreTy->isIntOrIntVectorTy())
       break;
 
     if (StoreTy->isPtrOrPtrVectorTy()) {
       StoreTy = Type::getIntNTy(F.getParent()->getContext(),
                                 DL.getTypeSizeInBits(StoreTy));
       break;
     }
   }
 
   unsigned Sz = DL.getTypeSizeInBits(StoreTy);
   unsigned AS = S0->getPointerAddressSpace();
   unsigned VecRegSize = TTI.getLoadStoreVecRegBitWidth(AS);
   unsigned VF = VecRegSize / Sz;
   unsigned ChainSize = Chain.size();
 
-  if (!isPowerOf2_32(Sz) || VF < 2 || ChainSize < 2)
+  if (!isPowerOf2_32(Sz) || VF < 2 || ChainSize < 2) {
+    InstructionsProcessed->insert(Chain.begin(), Chain.end());
     return false;
+  }
+
+  BasicBlock::iterator First, Last;
+  std::tie(First, Last) = getBoundaryInstrs(Chain);
+  unsigned StopChain = getVectorizablePrefixEndIdx(Chain, First, Last);
+  if (StopChain == 0) {
+    // There exists a side effect instruction, no vectorization possible.
+    InstructionsProcessed->insert(Chain.begin(), Chain.end());
+    return false;
+  }
+  if (StopChain == 1) {
+    // Failed after the first instruction. Discard it and try the smaller chain.
+    InstructionsProcessed->insert(Chain.front());
+    return false;
+  }
+
+  // Update Chain to the valid vectorizable subchain.
+  Chain = Chain.slice(0, StopChain);
+  ChainSize = Chain.size();
 
   // Store size should be 1B, 2B or multiple of 4B.
   // TODO: Target hook for size constraint?
   unsigned SzInBytes = (Sz / 8) * ChainSize;
   if (SzInBytes > 2 && SzInBytes % 4 != 0) {
     DEBUG(dbgs() << "LSV: Size should be 1B, 2B "
                     "or multiple of 4B. Splitting.\n");
     if (SzInBytes == 3)
-      return vectorizeStoreChain(Chain.slice(0, ChainSize - 1));
+      return vectorizeStoreChain(Chain.slice(0, ChainSize - 1),
+                                 InstructionsProcessed);
 
     auto Chains = splitOddVectorElts(Chain, Sz);
-    return vectorizeStoreChain(Chains.first) |
-           vectorizeStoreChain(Chains.second);
+    return vectorizeStoreChain(Chains.first, InstructionsProcessed) |
+           vectorizeStoreChain(Chains.second, InstructionsProcessed);
   }
 
   VectorType *VecTy;
   VectorType *VecStoreTy = dyn_cast<VectorType>(StoreTy);
   if (VecStoreTy)
     VecTy = VectorType::get(StoreTy->getScalarType(),
                             Chain.size() * VecStoreTy->getNumElements());
   else
     VecTy = VectorType::get(StoreTy, Chain.size());
 
   // If it's more than the max vector size, break it into two pieces.
   // TODO: Target hook to control types to split to.
   if (ChainSize > VF) {
     DEBUG(dbgs() << "LSV: Vector factor is too big."
                     " Creating two separate arrays.\n");
-    return vectorizeStoreChain(Chain.slice(0, VF)) |
-           vectorizeStoreChain(Chain.slice(VF));
+    return vectorizeStoreChain(Chain.slice(0, VF), InstructionsProcessed) |
+           vectorizeStoreChain(Chain.slice(VF), InstructionsProcessed);
   }
 
   DEBUG({
     dbgs() << "LSV: Stores to vectorize:\n";
     for (Value *V : Chain)
       V->dump();
   });
 
+  // We won't try again to vectorize the elements of the chain, regardless of
+  // whether we succeed below.
+  InstructionsProcessed->insert(Chain.begin(), Chain.end());
+
   // Check alignment restrictions.
   unsigned Alignment = getAlignment(S0);
 
   // If the store is going to be misaligned, don't vectorize it.
   if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
     if (S0->getPointerAddressSpace() != 0)
       return false;
 
     // If we're storing to an object on the stack, we control its alignment,
     // so we can cheat and change it!
     Value *V = GetUnderlyingObject(S0->getPointerOperand(), DL);
     if (AllocaInst *AI = dyn_cast_or_null<AllocaInst>(V)) {
       AI->setAlignment(TargetBaseAlign);
       Alignment = TargetBaseAlign;
     } else {
       return false;
     }
   }
 
-  BasicBlock::iterator First, Last;
-  std::tie(First, Last) = getBoundaryInstrs(Chain);
-
-  if (!isVectorizable(Chain, First, Last))
-    return false;
-
   // Set insert point.
   Builder.SetInsertPoint(&*Last);
 
   Value *Vec = UndefValue::get(VecTy);
 
   if (VecStoreTy) {
     unsigned VecWidth = VecStoreTy->getNumElements();
     for (unsigned I = 0, E = Chain.size(); I != E; ++I) {
bool Vectorizer::vectorizeStoreChain(
   SI->setAlignment(Alignment);
 
   eraseInstructions(Chain);
   ++NumVectorInstructions;
   NumScalarsVectorized += Chain.size();
   return true;
 }
 
-bool Vectorizer::vectorizeLoadChain(ArrayRef<Value *> Chain) {
+bool Vectorizer::vectorizeLoadChain(
+    ArrayRef<Value *> Chain, SmallPtrSet<Value *, 16> *InstructionsProcessed) {
   LoadInst *L0 = cast<LoadInst>(Chain[0]);
 
   // If the vector has an int element, default to int for the whole load.
   Type *LoadTy;
   for (const auto &V : Chain) {
     LoadTy = cast<LoadInst>(V)->getType();
     if (LoadTy->isIntOrIntVectorTy())
       break;
 
     if (LoadTy->isPtrOrPtrVectorTy()) {
       LoadTy = Type::getIntNTy(F.getParent()->getContext(),
                                DL.getTypeSizeInBits(LoadTy));
       break;
     }
   }
 
   unsigned Sz = DL.getTypeSizeInBits(LoadTy);
   unsigned AS = L0->getPointerAddressSpace();
   unsigned VecRegSize = TTI.getLoadStoreVecRegBitWidth(AS);
   unsigned VF = VecRegSize / Sz;
   unsigned ChainSize = Chain.size();
 
-  if (!isPowerOf2_32(Sz) || VF < 2 || ChainSize < 2)
+  if (!isPowerOf2_32(Sz) || VF < 2 || ChainSize < 2) {
+    InstructionsProcessed->insert(Chain.begin(), Chain.end());
+    return false;
+  }
+
+  BasicBlock::iterator First, Last;
+  std::tie(First, Last) = getBoundaryInstrs(Chain);
+  unsigned StopChain = getVectorizablePrefixEndIdx(Chain, First, Last);
+  if (StopChain == 0) {
+    // There exists a side effect instruction, no vectorization possible.
+    InstructionsProcessed->insert(Chain.begin(), Chain.end());
     return false;
+  }
+  if (StopChain == 1) {
+    // Failed after the first instruction. Discard it and try the smaller chain.
+    InstructionsProcessed->insert(Chain.front());
+    return false;
+  }
+
+  // Update Chain to the valid vectorizable subchain.
+  Chain = Chain.slice(0, StopChain);
+  ChainSize = Chain.size();
 
   // Load size should be 1B, 2B or multiple of 4B.
   // TODO: Should size constraint be a target hook?
   unsigned SzInBytes = (Sz / 8) * ChainSize;
   if (SzInBytes > 2 && SzInBytes % 4 != 0) {
     DEBUG(dbgs() << "LSV: Size should be 1B, 2B "
                     "or multiple of 4B. Splitting.\n");
     if (SzInBytes == 3)
-      return vectorizeLoadChain(Chain.slice(0, ChainSize - 1));
+      return vectorizeLoadChain(Chain.slice(0, ChainSize - 1),
+                                InstructionsProcessed);
     auto Chains = splitOddVectorElts(Chain, Sz);
-    return vectorizeLoadChain(Chains.first) | vectorizeLoadChain(Chains.second);
+    return vectorizeLoadChain(Chains.first, InstructionsProcessed) |
+           vectorizeLoadChain(Chains.second, InstructionsProcessed);
   }
 
   VectorType *VecTy;
   VectorType *VecLoadTy = dyn_cast<VectorType>(LoadTy);
   if (VecLoadTy)
     VecTy = VectorType::get(LoadTy->getScalarType(),
                             Chain.size() * VecLoadTy->getNumElements());
   else
     VecTy = VectorType::get(LoadTy, Chain.size());
 
   // If it's more than the max vector size, break it into two pieces.
   // TODO: Target hook to control types to split to.
   if (ChainSize > VF) {
     DEBUG(dbgs() << "LSV: Vector factor is too big. "
                     "Creating two separate arrays.\n");
-    return vectorizeLoadChain(Chain.slice(0, VF)) |
-           vectorizeLoadChain(Chain.slice(VF));
+    return vectorizeLoadChain(Chain.slice(0, VF), InstructionsProcessed) |
+           vectorizeLoadChain(Chain.slice(VF), InstructionsProcessed);
   }
 
+  // We won't try again to vectorize the elements of the chain, regardless of
+  // whether we succeed below.
+  InstructionsProcessed->insert(Chain.begin(), Chain.end());
+
   // Check alignment restrictions.
   unsigned Alignment = getAlignment(L0);
 
   // If the load is going to be misaligned, don't vectorize it.
   if (accessIsMisaligned(SzInBytes, AS, Alignment)) {
     if (L0->getPointerAddressSpace() != 0)
       return false;
 
bool Vectorizer::vectorizeLoadChain(
   }
 
   DEBUG({
     dbgs() << "LSV: Loads to vectorize:\n";
     for (Value *V : Chain)
       V->dump();
   });
 
-  BasicBlock::iterator First, Last;
-  std::tie(First, Last) = getBoundaryInstrs(Chain);
-
-  if (!isVectorizable(Chain, First, Last))
-    return false;
-
   // Set insert point.
   Builder.SetInsertPoint(&*First);
 
   Value *Bitcast =
       Builder.CreateBitCast(L0->getPointerOperand(), VecTy->getPointerTo(AS));
 
   LoadInst *LI = cast<LoadInst>(Builder.CreateLoad(Bitcast));
   propagateMetadata(LI, Chain);

llvm/trunk/test/Transforms/LoadStoreVectorizer/X86/subchain-interleaved.ll

define void @interleave_3L_2S_1L(i32* noalias %ptr) {
   %l4 = load i32, i32* %next.gep2, align 4
 
   ret void
 }
 
 ; CHECK-LABEL: @chain_suffix(
 ; CHECK: load i32
 ; CHECK: store <2 x i32>
-; CHECK: load i32
-; CHECK: load i32
+; CHECK: load <2 x i32>
 define void @chain_suffix(i32* noalias %ptr) {
   %next.gep = getelementptr i32, i32* %ptr, i64 0
   %next.gep1 = getelementptr i32, i32* %ptr, i64 1
   %next.gep2 = getelementptr i32, i32* %ptr, i64 2
 
   %l2 = load i32, i32* %next.gep, align 4
   store i32 0, i32* %next.gep1, align 4
   store i32 0, i32* %next.gep, align 4
   %l3 = load i32, i32* %next.gep1, align 4
   %l4 = load i32, i32* %next.gep2, align 4
 
   ret void
 }
 
 
 ; CHECK-LABEL: @chain_prefix_suffix(
-; CHECK: load i32
-; CHECK: load i32
+; CHECK: load <2 x i32>
 ; CHECK: store <2 x i32>
-; CHECK: load i32
-; CHECK: load i32
-; CHECK: load i32
+; CHECK: load <3 x i32>
 define void  @chain_prefix_suffix(i32* noalias %ptr) {
   %next.gep = getelementptr i32, i32* %ptr, i64 0
   %next.gep1 = getelementptr i32, i32* %ptr, i64 1
   %next.gep2 = getelementptr i32, i32* %ptr, i64 2
   %next.gep3 = getelementptr i32, i32* %ptr, i64 3
 
   %l1 = load i32, i32* %next.gep, align 4
   %l2 = load i32, i32* %next.gep1, align 4