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

[Serge Pavlov]

  Thank you for review!


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2073
@@ +2072,3 @@
+          if (CI)
+            I.swapOperands();
+          else
----------------
Benjamin Kramer wrote:
> 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.
Removed this code.

================
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 &&
----------------
Benjamin Kramer wrote:
> You can use dyn_cast<TruncInst> directly
Overlooked this opportunity. Fixed.

================
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;
----------------
Benjamin Kramer wrote:
> if chains like this can be represented with match() in a much clearer way.
Changed this piece of code using match().

================
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 {
----------------
Benjamin Kramer wrote:
> This feels like an awful lot of code for this. Can't you just RAUW the value and have instcombine clean up after you?
This code not only RAUW new value, it also adapt the value using ZExt, because new multiplication result is short, but users still wait long value, although high bits are ignored.


http://reviews.llvm.org/D2814