[PATCH] D62432: [SLPVectorizer] Make the scheduler aware of the TreeEntry operands.

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1509-1511
   TreeEntry *newTreeEntry(ArrayRef<Value *> VL, bool Vectorized,
                           const EdgeInfo &UserTreeIdx,
+                          ScheduleData *SchedBundle,
----------------
ABataev wrote:
> Seems to me, the values of `Vectorized` and `SchedBundle` params are tied: `SchedBundle` cannot be `nullptr` if `Vectorized` is `true`. Can we exclude `Vectorized` in this case?
No, they are not tied: PHIs don't get scheduled, so they get no bundle, but a TreeEntry is generated for them. 
Also we cannot generalize this rule: having PHInodes does not guarantee that we can create a vectorizable TreeEntry. For example, if these PHIs are already in tree, we would need to gather.

So we cannot just use a single variable, like the bundle pointer, to represent this state. I wrapped the bundle pointer inside an `Optional` as @RKSimon suggested, which can carry the state we need.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2490
       LLVM_DEBUG(dbgs() << "SLP: Gather extract sequence.\n");
-      newTreeEntry(VL, /*Vectorized=*/false, UserTreeIdx, ReuseShuffleIndicies);
+      newTreeEntry(VL, /*Vectorized=*/false, UserTreeIdx, Bundle,
+                   ReuseShuffleIndicies);
----------------
ABataev wrote:
> Do you really need to provide `Bundle` here for gathering node?
Good catch, no need to provide bundle here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62432/new/

https://reviews.llvm.org/D62432