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

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov added inline comments.


================
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.
----------------
RKSimon wrote:
> 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.
Correct, Thanks.


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


================
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");
----------------
RKSimon wrote:
> weird indentation - clang-format ?
oh, thanks.


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


https://reviews.llvm.org/D36518