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

[Jingyue Wu]

Only one nit, otherwise LGTM.

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:515
@@ +514,3 @@
+
+      cast<BinaryOperator>(SimplifiedInst)->setHasNoSignedWrap(HasNSW);
+    }
----------------
Dinesh Dwivedi wrote:
> Jingyue Wu wrote:
> > I may be over-concerned, but is this cast dangerous? CreateBinOp returns a ConstantExpr if its operands are both Constant. Note that ConstantInt op ConstantInt is not always foldable, e.g., ptrtoint (a global variable) + 5. 
> We have already check if 'isa<OverflowingBinaryOperator>(SimplifiedInst)' so casting SimplifiedInst to BinaryOperator should be safe.
http://llvm.org/docs/doxygen/html/classllvm_1_1OverflowingBinaryOperator.html

OverflowingBinaryOperator is not a subclass of BinaryOperator. BinaryOperator is an Instruction, but OverflowingBinaryOperator can be either an Instruction or a ConstantExpr. 

I agree the naming is a little confusing :)

http://reviews.llvm.org/D3799