[PATCH] D37019: Add select simplifications

[David Majnemer via Phabricator via llvm-commits]

majnemer accepted this revision.
majnemer added a comment.

LGTM with nits addressed.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:714
+
+Value *InstCombiner::SimplifySelectHelper(BinaryOperator &I, Value *LHS,
+                                          Value *RHS, bool CarryFastMathFlags) {
----------------
I'd rename it something a little more unique/descriptive like `SimplifySelectsFeedingBinaryOp`


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:727
+      Value *V1;
+      if ((V1 = SimplifyBinOp(Opcode, C, E, SQ.getWithInstruction(&I)))) 
+        SI = Builder.CreateSelect(A1, Builder.CreateBinOp(Opcode, B, D), V1);
----------------
Could you assign this on the prior line? I don't think you save anything by doing the assignment in the if statement because the variable is used later.


https://reviews.llvm.org/D37019