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

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie created this revision.
vdmitrie added reviewers: ABataev, Vasilis.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75296

Files:
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp


Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -2016,6 +2016,20 @@
         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]))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75296.247079.patch
Type: text/x-patch
Size: 1341 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200227/cc81f1d3/attachment.bin>