[PATCH] D28907: [SLP] Fix for PR30787: Failure to beneficially vectorize 'copyable' elements in integer binary ops.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3244
         isJumbled = true;
-        LI = cast<LoadInst>(E->Scalars[0]);
-      } else {
-        LI = cast<LoadInst>(VL0);
-      }
+      LI = cast<LoadInst>(E->Scalars[0]);
 
----------------
dtemirbulatov wrote:
> ABataev wrote:
> > dtemirbulatov wrote:
> > > ABataev wrote:
> > > > dtemirbulatov wrote:
> > > > > We used E->VL0 here before, which is not correct because it could point to a wrong element.
> > > > Why could it point to a wrong element? As I understand, it should always point to Scalars[0] for LoadInst.
> > > No, it is in order as it was discovered. I will add a testcase for the issue.
> > I still don't understand what's the problem here. Why we can't use `VL0 ` in both cases?
> For consecutive loads [load0, load1, load2, load3] in HorizontalReduction::matchAssociativeReduction function ReducedVals vector was formed as  [load1, load0, load2, load3] and later TreeEntry was added with State.OpValue pointing to load1 in BoUpSLP::buildTree_rec, while TreeEntry->Scalars contained correct sequence of loads [load0, load1, load2, load3].
Could you investigate why does this happen?


https://reviews.llvm.org/D28907