[PATCH] D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.

Nice!
I like this.



================
Comment at: include/llvm/IR/Instructions.h:2471
+    assert(Mask->getType()->isVectorTy() && "Shuffle needs vector constant.");
+    SmallVector<int, 16> MaskAsInts;
+    getShuffleMask(Mask, MaskAsInts);
----------------
spatel wrote:
> lebedev.ri wrote:
> > From where does this 16 come from?
> > Magical expected maximal number of elements in one vector? (AVX512 / 16 elements = 32 bit/element)
> > Might be better to `static constexpr int` it beforehand.
> Yes - copy/paste of magic constant. This is typical SmallVector usage throughout LLVM. OK, if we make that a follow-up cleanup and zap all of them (at least in this file)?
Yep, absolutely, just pointing out.


================
Comment at: lib/IR/Instructions.cpp:1691
+  bool UsesRHS = false;
+  for (int i = 0, NumElts = Mask.size(); i < NumElts; ++i) {
+    if (Mask[i] == -1)
----------------
Oh, you can define multiple variables in init of the `for` loop?
Somehow i never realized that..


https://reviews.llvm.org/D48236