[PATCH] [Instcombine] Recognize test for overflow in integer multiplication.

[Benjamin Kramer]

  Been really busy over the last couple of weeks, sorry for the latency. Here are some preliminary comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2073
@@ +2072,3 @@
+          if (CI)
+            I.swapOperands();
+          else
----------------
Not sure why you have this code path in here. An and with a constant on the LHS should've been canonicalized already before instcombine gets here.

You also have to be really careful when modifying instructions like this if you don't want to confuse the rest of instcombine.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2096
@@ +2095,3 @@
+        Value *ZextArg = ZextInstr->getOperand(0);
+        if (Instruction *TruncInstr = dyn_cast<Instruction>(ZextArg))
+          if (TruncInstr->getOpcode() == Instruction::Trunc &&
----------------
You can use dyn_cast<TruncInst> directly

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2108
@@ +2107,3 @@
+      if (BO->getOpcode() == Instruction::And)
+        if (ConstantInt *CI = dyn_cast<ConstantInt>(BO->getOperand(1))) {
+          APInt CVal = CI->getValue() + 1;
----------------
if chains like this can be represented with match() in a much clearer way.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2189-2209
@@ +2188,23 @@
+  // with the new mul.
+  if (MulVal->hasNUsesOrMore(2)) {
+    Value *Mul = Builder->CreateExtractValue(Call, 0, "umul.value");
+    for (User *U : MulVal->users()) {
+      if (U == &I || U == OtherVal)
+        continue;
+      if (TruncInst *TI = dyn_cast<TruncInst>(U)) {
+        if (TI->getType()->getPrimitiveSizeInBits() == MulWidth)
+          IC.ReplaceInstUsesWith(*TI, Mul);
+        else
+          TI->setOperand(0, Mul);
+      } else if (BinaryOperator *BO = dyn_cast<BinaryOperator>(U)) {
+        assert(BO->getOpcode() == Instruction::And);
+        // Replace (mul & mask) --> zext (mul.with.overflow & short_mast)
+        ConstantInt *CI = cast<ConstantInt>(BO->getOperand(1));
+        APInt Mask = CI->getValue();
+        Mask = Mask.trunc(MulWidth);
+        Value *ShortAnd = Builder->CreateAnd(Mul, Mask);
+        Instruction *Zext =
+            cast<Instruction>(Builder->CreateZExt(ShortAnd, BO->getType()));
+        IC.Worklist.Add(Zext);
+        IC.ReplaceInstUsesWith(*BO, Zext);
+      } else {
----------------
This feels like an awful lot of code for this. Can't you just RAUW the value and have instcombine clean up after you?

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3123
@@ +3122,3 @@
+    if (match(Op0, m_Mul(m_ZExt(m_Value(A)), m_ZExt(m_Value(B))))) {
+      if (isa<IntegerType>(A->getType())) {
+        if (Instruction *R = ProcessUMulZExtIdiom(I, Op0, Op1, *this))
----------------
zext only works on integers no need for an extra check.


http://reviews.llvm.org/D2814