[llvm-commits] [llvm] r122183 - in /llvm/trunk/lib/Transforms/InstCombine: InstCombineCalls.cpp InstructionCombining.cpp

[Frits van Bommel]

On Sun, Dec 19, 2010 at 8:43 PM, Chris Lattner <sabre at nondot.org> wrote:
> -    // If the normal result of the add is dead, and the RHS is a constant, we
> -    // can transform this into a range comparison.
> -    // overflow = uadd a, -4  -->  overflow = icmp ugt a, 3
> -    if (ConstantInt *CI = dyn_cast<ConstantInt>(RHS))
> -      if (ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(II->use_back()))
> -        if (II->hasOneUse() && EVI->getNumIndices() == 1 && !EVI->use_empty() &&
> -            *EVI->idx_begin() == 1) {  // Extract of overflow result.
> -          Builder->SetInsertPoint(EVI);
> -          Value *R = Builder->CreateICmpUGT(LHS, ConstantExpr::getNot(CI));
> -          R->takeName(EVI);
> -          ReplaceInstUsesWith(*EVI, R);
> -          return II;
> -        }
> -

This only handles uadd.with.overflow(), but

> +
> +        // If the normal result of the add is dead, and the RHS is a constant,
> +        // we can transform this into a range comparison.
> +        // overflow = uadd a, -4  -->  overflow = icmp ugt a, 3
> +        if (ConstantInt *CI = dyn_cast<ConstantInt>(II->getArgOperand(1)))
> +          return new ICmpInst(ICmpInst::ICMP_UGT, II->getArgOperand(0),
> +                              ConstantExpr::getNot(CI));

this also runs on sadd.with.overflow(). This seems fishy to me.
Shouldn't you at least use a different predicate for that one?