[PATCH] D48401: [InstCombine] fold vector select of binops with constant ops to 1 binop (PR37806)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1152
+
+  if (B0->getOpcode() == B1->getOpcode()) {
+    Value *X;
----------------
lebedev.ri wrote:
> Do you envision adding other top-level `if`'s later?
> Otherwise you could do early return.
Yes, there are 2 potential near-term follow-ups discussed in the bug report:
1. Match 2 variable vectors rather than 1 repeated vector.
2. Match different opcodes with special constants (example: mul and shl).

Assuming these are all good combines, they might be big enough that they each deserve their own helper, so I'll adjust this.


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1170-1171
+      // binop to be undef.
+      if (B0->isIntDivRem())
+        NewC = getSafeVectorConstantForIntDivRem(NewC);
+
----------------
lebedev.ri wrote:
> Hm, you are sure there is a test for this in `test/Transforms/InstCombine/shuffle_select.ll`?
> 
Yes - the sdiv and urem tests have undef elements in the shuffle mask, so they would both fail without this condition.


https://reviews.llvm.org/D48401