[PATCH] D16691: [InstCombine] simplify masked load intrinsics with constant masks

[Simon Pilgrim via llvm-commits]

RKSimon added a comment.

Some minor thoughts - I'd like to see what others have to say as well though.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:690
@@ +689,3 @@
+         "Wrong type for 4th arg");
+
+  Value *LoadPtr = II.getArgOperand(0);
----------------
Minor: You can relax the type requirements by using Constant::isNullValue() and Constant::isAllOnesValue()

But this is true for a lot of the code in InstCombineCalls.cpp .....

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:708
@@ +707,3 @@
+  // insertelement into the passthru vector.
+
+  return nullptr;
----------------
I'm really not sure whether we should try to do this here or leaving it until lowering. Its making a big assumption that the target is good at scalar loads + insertions.


http://reviews.llvm.org/D16691