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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1509-1511
   TreeEntry *newTreeEntry(ArrayRef<Value *> VL, bool Vectorized,
                           const EdgeInfo &UserTreeIdx,
+                          ScheduleData *SchedBundle,
----------------
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?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1511
                           const EdgeInfo &UserTreeIdx,
+                          ScheduleData *SchedBundle,
                           ArrayRef<unsigned> ReuseShuffleIndices = None,
----------------
Set default value to `= nullptr` to reduce the number of changes in `newTreeEntry` function calls.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1867
+        // Decrement the unscheduled counter and insert to ready list if ready.
+        auto decrUnsched = [&](Instruction *I) {
           doForAllOpcodes(I, [&ReadyList](ScheduleData *OpDef) {
----------------
Better to use an explicit list of captures.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1896-1898
+        // If BundleMember is a stand-alone instruction, no operand reordering
+        // has taken place, so we directly access its operands.
+        else {
----------------
The formatting here is very strange, better to put comment inside of the `else` block.


================
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);
----------------
Do you really need to provide `Bundle` here for gathering node?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62432