[PATCH] D36518: [SLPVectorizer] Schedule bundle with different opcodes.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Test cases?



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:970
       while (BundleMember) {
-        // Handle the def-use chain dependencies.
-        for (Use &U : BundleMember->Inst->operands()) {
-          ScheduleData *OpDef = getScheduleData(U.get());
-          if (OpDef && OpDef->hasValidDependencies() &&
-              OpDef->incrementUnscheduledDeps(-1) == 0) {
-            // There are no more unscheduled dependencies after decrementing,
-            // so we can put the dependent instruction into the ready list.
-            ScheduleData *DepBundle = OpDef->FirstInBundle;
-            assert(!DepBundle->IsScheduled &&
-                   "already scheduled bundle gets ready");
-            ReadyList.insert(DepBundle);
-            DEBUG(dbgs() << "SLP:    gets ready (def): " << *DepBundle << "\n");
+        if (BundleMember->Inst == BundleMember->OpValue) {
+          // Handle the def-use chain dependencies.
----------------
You can avoid the extra indentation by an early-out
```
if (BundleMember->Inst != BundleMember->OpValue) {
  BundleMember = BundleMember->NextInBundle;
  continue;
}
```

You might even be able to tidily replace the while loop with a for loop it you used shorter variable names.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3439
+  auto &&CheckSheduleForI = [this, OpValue](Instruction *I) -> bool {
+    if (ScheduleData *ISD = getScheduleData(I)) {
+      assert(isInSchedulingRegion(ISD) &&
----------------
early-out:
```
ScheduleData *ISD = getScheduleData(I);
if (!ISD)
  return false;
```
you can then remove that (void)ISD;


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3459
+    if (isOneOf(OpValue, I) != I)
+                    CheckSheduleForI(I);
     assert(ScheduleEnd && "tried to vectorize a TerminatorInst?");
----------------
weird indentation - clang-format ?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3482
+        if (isOneOf(OpValue, I) != I)
+                        CheckSheduleForI(I);
         DEBUG(dbgs() << "SLP:  extend schedule region start to " << *I << "\n");
----------------
weird indentation - clang-format ?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3494
+        if (isOneOf(OpValue, I) != I)
+                        CheckSheduleForI(I);
         assert(ScheduleEnd && "tried to vectorize a TerminatorInst?");
----------------
weird indentation - clang-format ?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3570
+        if (BundleMember->OpValue != BundleMember->Inst) {
+          ScheduleData *UseSD = getScheduleData(BundleMember->Inst);
+          if (UseSD && isInSchedulingRegion(UseSD->FirstInBundle)) {
----------------
Comments! You used "Handle def-use chain dependencies." for both cases!


https://reviews.llvm.org/D36518