[PATCH] Strength reduce intrinsics with overflow into regular arithmetic operations if possible.

[Philip Reames]

In the future, please split your changes into the smallest chunks you can.  In this case, you've got three pieces:
- The refactoring to draw out the factory function - LGTM
- The signed and unsigned add cases with existing functions - LGTM
- The signed multiplication case with a new helper function - Needs a second opinion.  I don't spot anything obviously wrong, but I don't trust my own reasoning around overflow.  I suspect you could also use this helper function in the normal signed mul case as well.  

If you want to split it for submission and only hold the last feel free.

================
Comment at: lib/Transforms/InstCombine/InstCombine.h:337
@@ +336,3 @@
+  /// \p Result's name is taken from \p II.
+  Instruction *CreateOverflowResult(IntrinsicInst *II, Value *Result,
+                                    bool Overflow, bool ReUseName = true) {
----------------
Nit: CreateOverflowResultTuple or CreateOverflowTuple

extremely minor, feel free to ignore if you disagree

================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1017
@@ +1016,3 @@
+    unsigned BitWidth = IT->getBitWidth();
+    unsigned SignBits = ComputeNumSignBits(LHS, 0, CxtI) +
+                        ComputeNumSignBits(RHS, 0, CxtI);
----------------
Brief comment here would be good.  You appear to be using the fact that a N bit number multiplied by a M bit number can yield a maximum of an N+M+1 bit number right?

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:372
@@ -371,10 +371,3 @@
         // Create a simple add instruction, and insert it into the struct.
-        Value *Add = Builder->CreateAdd(LHS, RHS);
-        Add->takeName(&CI);
-        Constant *V[] = {
-          UndefValue::get(LHS->getType()),
-          ConstantInt::getTrue(II->getContext())
-        };
-        StructType *ST = cast<StructType>(II->getType());
-        Constant *Struct = ConstantStruct::get(ST, V);
-        return InsertValueInst::Create(Struct, Add, 0);
+        return CreateOverflowResult(II, Builder->CreateAdd(LHS, RHS), true,
+                                    /*ReUseName*/true);
----------------
General comment: Doing refactorings in separate changes from semantic changes makes it much easier and faster to review.  Strongly, strongly preferred.

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:439
@@ -461,1 +438,3 @@
+    }
     break;
+  }
----------------
This change is clearly fine.  

As a general point, it feels like this code could be factored to share a lot more of the logic from the normal arithmetic cases.  However, if you do that, it should be a separate change.

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:494
@@ -521,1 +493,3 @@
+      }
+    }
     break;
----------------
This change is the one I'm not entirely sure of - due to the implementation of the helper function - and would like a second opinion on.

http://reviews.llvm.org/D6477