[PATCH] D57779: [SLP] Add support for throttling.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:612
+  };
+  using TEVectorizableSet = std::set<TreeEntry *, TECostComparator>;
+
----------------
`PriorityQueue`?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5815-5824
+  for (TreeEntry *Entry : RemovedOperations) {
+    ScheduleData *SD = BS->getScheduleData(Entry->Scalars[0]);
+    if (SD && SD->isPartOfBundle()) {
+      if (!Removed) {
+        Removed = true;
+        BS->resetSchedule();
+      }
----------------
Looks like you need to implement something like `reduceSchedulingRegion()`, similar to `extendSchedulingRegion` function. Because otherwise you're going to operate with the larger scheduling region. I.e. need to modify `ScheduleStart` and `ScheduleEnd` data members.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6680
         Changed = true;
-      }
+      } else if (SLPThrottleBudget > 0 &&
+                 R.getTreeCost(true, UserCost) < -SLPCostThreshold) {
----------------
Why `SLPThrottleBudget > 0`? What if `SLPThrottleBudget` equals 0?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6680-6681
         Changed = true;
-      }
+      } else if (SLPThrottleBudget > 0 &&
+                 R.getTreeCost(true, UserCost) < -SLPCostThreshold) {
+        R.vectorizeTree();
----------------
ABataev wrote:
> Why `SLPThrottleBudget > 0`? What if `SLPThrottleBudget` equals 0?
Why we can't do something like this:
```
int NumAttempts = 0;
do {
      if (R.isTreeTinyAndNotFullyVectorizable())
        break;

      R.computeMinimumValueSizes();
      InstructionCost Cost = R.getTreeCost();
      InstructionCost UserCost = 0;
      ....
      if (Cost < -SLPCostThreshold) {
        LLVM_DEBUG(dbgs() << "SLP: Vectorizing list at cost:" << Cost << ".\n");
        R.getORE()->emit(OptimizationRemark(SV_NAME, "VectorizedList",
                                                    cast<Instruction>(Ops[0]))
                                 << "SLP vectorized with cost " << ore::NV("Cost", Cost)
                                 << " and with tree size "
                                 << ore::NV("TreeSize", R.getTreeSize()));

        R.vectorizeTree();
        // Move to the next bundle.
        I += VF - 1;
        NextInst = I + 1;
        Changed = true;
        break;
      }
   ...
   /// Do throttling here.
   ++NumAttempts;
} while (NumAttempts < SLPThrottleBudget);
```


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

https://reviews.llvm.org/D57779