[PATCH] D51781: [InstCombine] Do not fold scalar ops over select with vector condition.

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM - see inline for some minor bits.



================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:365
 
+  // If the select condition is a vector, the operands also must be vectors.
+  if (SI.getCondition()->getType()->isVectorTy() &&
----------------
I found this comment confusing. The select operands themselves are vectors, but the operands of those operands must also be vectors. This can only be a problem with GEPs? If that's correct, let's state that in the comment.


================
Comment at: test/Transforms/InstCombine/select-gep.ll:141
+
+define <2 x i64*> @test5(i64* %p, <2 x i1> %cc) {
+; CHECK-LABEL: @test5(
----------------
We could reduce this a bit I think:

```
define <2 x i64*> @test5(i64* %gep1, i64* %gep2, <2 x i1> %cc) {
  %gep3 = getelementptr i64, i64* %gep1, <2 x i64> undef
  %gep4 = getelementptr i64, i64* %gep2, <2 x i64> undef
  %select = select <2 x i1> %cc, <2 x i64*> %gep3, <2 x i64*> %gep4
  ret <2 x i64*> %select
}

```


https://reviews.llvm.org/D51781