[SLP][NFC] Assert that tree entry operands completed when scheduler looks for dependencies.

[vdmitrie]

Summary

This change adds an assertion to prevent tricky bug related to recursive
approach of building vectorization tree. For loop below takes number of
operands directly from tree entry rather than from scalars.
If the entry at this moment turns out incomplete (i.e. not all operands set)
then not all the dependencies will be seen by the scheduler.
This situation can lead to failed scheduling (and thus failed vectorization).
Here is code example originating it:

for (i : VL0->getNumOperands()) {
  ...
  TE->setOperand(i, Operands);
  buildTree_rec(Operands, Depth + 1,...);
}

Correct way is two steps process: first set all operand to the tree entry and
then recursively process each operand.

[ABataev]

Maybe it is better to implement the two-steps approach? If it may lead to a crash, it will definitely cause a crash somewhere

[vdmitrie]

In D75296#1896620, @ABataev wrote:

Maybe it is better to implement the two-steps approach? If it may lead to a crash, it will definitely cause a crash somewhere

I'm not sure I got your question. The two steps approach is what is implemented and used at the moment. Example in the summary shows situation leading to firing the assertion.

[ABataev]

In D75296#1896654, @vdmitrie wrote:

In D75296#1896620, @ABataev wrote:

Maybe it is better to implement the two-steps approach? If it may lead to a crash, it will definitely cause a crash somewhere

I'm not sure I got your question. The two steps approach is what is implemented and used at the moment. Example in the summary shows situation leading to firing the assertion.

SO, we have correct implementation and this is just a check to prevent possible bugs in the future?

[vdmitrie]

SO, we have correct implementation and this is just a check to prevent possible bugs in the future?

Right.

[ABataev]

LGTM

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

void schedule(ScheduleData *SD, ReadyListType &ReadyList) {
         };
 
         // If BundleMember is a vector bundle, its operands may have been
         // reordered duiring buildTree(). We therefore need to get its operands
         // through the TreeEntry.
         if (TreeEntry *TE = BundleMember->TE) {
           int Lane = BundleMember->Lane;
           assert(Lane >= 0 && "Lane not set");
+
+          // Since vectorization tree is being built recursively this assertion
+          // ensures that the tree entry has all operands set before reaching
+          // this code. Couple of exceptions known at the moment are extracts
+          // where their second (immediate) operand is not added. Since
+          // immediates do not affect scheduler behavior this is considered
+          // okay.
+          auto *In = TE->getMainOp();
+          assert(In &&
+                 (isa<ExtractValueInst>(In) || isa<ExtractElementInst>(In) ||
+                  In->getNumOperands() == TE->getNumOperands()) &&
+                 "Missed TreeEntry operands ?");
+          (void)In; // fake use to avoid build failure when assertions disabled
+
           for (unsigned OpIdx = 0, NumOperands = TE->getNumOperands();
                OpIdx != NumOperands; ++OpIdx)
             if (auto *I = dyn_cast<Instruction>(TE->getOperand(OpIdx)[Lane]))
               DecrUnsched(I);
         } else {
           // If BundleMember is a stand-alone instruction, no operand reordering
           // has taken place, so we directly access its operands.
           for (Use &U : BundleMember->Inst->operands())