[PATCH] D115653: [DAG]Introduce llvm::processShuffleMasks and use it for shuffles in DAG Type Legalizer.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I've read this patch a few times now but don't think I understand it enough to give great feedback I'm afraid, other than can we try and explain the details in the "2 or more" case a little more? This constructs a triple nested vector, most of which is expected to be empty?

I like the idea of sharing processShuffleMasks between the codegen and costmodel. That sounds like a nice way of doing things.



================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:531
+  }
+  // Process splitted mask.
+  for (unsigned I = 0; I < NumOfUsedRegs; ++I) {
----------------
I don't think splitted is a word. I think it would just be split, if my english isn't letting me down.


================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:550-552
+      // The first mask is a permutation of a single register, which can
+      // be transformed into a shuffle of 2 registers instead of 1 reg
+      // shuffle + 2 reg shuffles in case if we have 2+ input vectors.
----------------
I'm not sure I understand this code very well, and the comment isn't very clear to me. Can you try and expand the comments a bit? And maybe giving `It` a better name, that kind of thing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115653/new/

https://reviews.llvm.org/D115653