[PATCH] D26083: [LV] Scalarize operands of predicated instructions

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

This LGTM, except some vague thoughts about slightly relaxing canBeScalarized().
If you think that's not a real concern, I'm ok with this going on as is.



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6627
+    if (!I->hasOneUse() || PredInst->getParent() != I->getParent() ||
+        Legal->isScalarAfterVectorization(I))
+      return false;
----------------
Please add an explanation for why we bail on Legal->isScalarAfterVectorization(I)


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6641
+      if (auto *J = dyn_cast<Instruction>(U.get()))
+        if (Legal->isUniformAfterVectorization(J))
+          return false;
----------------
Are you sure this is the right check?

If I understand correctly, we fail not because the pointer is uniform, but because it's only uniform in the "we use a single (LLVM) value to represent it" sense, not the "the (abstract) value is the same for all lanes" sense. Otherwise it'd be safe to scalarize.

I'm having a hard time to think of a good example, though because this is limited to instructions (so GV and param operands won't be affected), and uniform instructions tend to either be consecutive, loop-invariant (in which case they should be hoisted out by LICM before we hit the vectorizer), or uses of the scalar IV (in which case they won't be operands of vectorized instructions).


https://reviews.llvm.org/D26083