[PATCH] D32863: InstructionSimplify: Relanding r301766

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4087-4088
+
+  // Canonicalization: if only one input vector is constant, it shall be the
+  // second one.
+  if (Op0Const && !Op1Const) {
----------------
Is the plan to actually do this canonicalization of constant to 2nd operand in InstCombine? If so, wouldn't it make sense to add a "commute" or "commuteMask" (this is what it's called in the DAG) function directly to ShuffleVectorInst and then use that here?


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4091
+    std::swap(Op0, Op1);
+    for (auto &Idx : Indices) {
+      if (Idx == -1)
----------------
Use 'int &' instead of 'auto &'.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4094
+        continue;
+      Idx = Idx < (int)InVecNumElts ? Idx + InVecNumElts : Idx - InVecNumElts;
+    }
----------------
zvi wrote:
> This line was replaced to fix the typo in D32338:
> 
> ```
> Idx = Idx < (int)MaskNumElts ? Idx + MaskNumElts : Idx - MaskNumElts;
> ```
> 
assert(Idx >= 0 && Idx < InVecNumElts * 2) ?


https://reviews.llvm.org/D32863