[PATCH] D26905: [SLP] Vectorize loads of consecutive memory accesses, accessed in non-consecutive (jumbled) way.

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2575-2576
+      if (E->NeedToShuffle) {
+        assert(VL.size() == E->Scalars.size() &&
+               "Equal number of scalars expected");
+        SmallVector<Constant *, 8> Mask;
----------------
ashahid wrote:
> mssimpso wrote:
> > Hi Shahid,
> > 
> > I'm hitting the assertion here while testing this patch. Can you take a look?
> Sure. If possible can you share the asserting test?
Sure, I'll try and reduce something for you.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2596
+        if (Instruction *I = dyn_cast<Instruction>(shuf))
+          return propagateMetadata(I, VL);
+        return shuf;
----------------
ashahid wrote:
> mssimpso wrote:
> > I also saw verifier failures where TBAA metadata had been applied to the shuffle, like:
> > 
> > ```
> > TBAA is only for loads, stores and calls!
> >   %14 = shufflevector <4 x i32> %13, <4 x i32> undef, <4 x i32> <i32 2, i32 3, i32 0, i32 1>, !tbaa !66
> > ```
> Ok, will fix it.
I think you should probably just copy the metadata from the scalar load to the vector load, like:

```
propagateMetadata(LI, E->Scalars);
return Shuf;
```




https://reviews.llvm.org/D26905