[PATCH] Fixing inst-combine not to drops nsw when combining adds into mul (PR19263)

[Rafael Ávila de Espíndola]

Mostly nits. This is LGTM with the issues on line 552 addressed, but please wait for Jingyue Wu to confirm that he is OK with it too.

Thanks!

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:415
@@ +414,3 @@
+  }
+  return nullptr;
+}
----------------
You have both a default case with a return and a return after the switch.

Given that we only handle Mul, it is probably better to write this as an if for now. When we add support for other opcodes it is easy to go back to an switch.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:552
@@ +551,3 @@
+  if (Value *V = tryFactorization(Builder, DL, I, RHSOpcode, LHS,
+                                  getIdentityValue(LHSOpcode, RHS), C, D))
+    return V;
----------------
This should be

  if (Value *V = tryFactorization(Builder, DL, I, RHSOpcode, LHS,
                                  getIdentityValue(LHSOpcode, LHS), C, D))

no?

http://reviews.llvm.org/D3799