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

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 29 07:37:25 PDT 2019


RKSimon added a comment.

Some minors, @ABataev any thoughts?



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1438
+      assert(Operands.empty() && "Already initialized?");
+      Instruction *I0 = dyn_cast<Instruction>(Scalars[0]);
+      assert(I0 && "Expected instruction.");
----------------
auto, also we're asserting for I0 != null below - so maybe just use cast<> and drop the assert?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1446
+        for (unsigned Lane = 0; Lane != NumLanes; ++Lane) {
+          Instruction *I = dyn_cast<Instruction>(Scalars[Lane]);
+          assert(I && I->getNumOperands() == NumOperands &&
----------------
auto, also we're asserting for I != null below - so maybe just use cast<> and just assert for getNumOperands?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1947
     /// cyclic dependencies. This is only a dry-run, no instructions are
     /// actually moved at this stage.
+    std::pair<bool, ScheduleData *>
----------------
Update description to explain the returned values?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2399
+  ScheduleData *Bundle;
+  std::tie(Success, Bundle) = BS.tryScheduleBundle(VL, this, S);
+  if (!Success) {
----------------
Use Optional instead?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62432





More information about the llvm-commits mailing list