[PATCH] D32338: InstructionSimplify: Canonicalize shuffle operands. NFC-ish.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4148
   for (unsigned i = 0; i != MaskNumElts; ++i) {
     int Idx = ShuffleVectorInst::getMaskValue(Mask, i);
     if (Idx == -1)
----------------
You are calling ShuffleVectorInst::getMaskValue multiple times per element - better to call ShuffleVectorInst::getShuffleMask instead?


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4169
+  bool MaskHasAnyUndefs = false;
+  // Canonicalization: if only one input vector is contant, it shall be the
+  // second one.
----------------
constant


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4170
+  // Canonicalization: if only one input vector is contant, it shall be the
+  // second one.
+  if (Op0Const && !Op1Const) {
----------------
Add tests for this?


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4197
+    return nullptr;
 
   // Check if every element of this shuffle can be mapped back to the
----------------
Add an assert here to check that NewIndices has MaskNumElts elements


Repository:
  rL LLVM

https://reviews.llvm.org/D32338