[llvm-commits] [llvm] r112550 - /llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp

[Chris Lattner]

On Aug 30, 2010, at 4:22 PM, Owen Anderson wrote:
> Re-apply r112539, being more careful to respect the return values of the constant folding methods.  Additionally,
> use the ConstantExpr::get*() methods to simplify some constant folding.

Thanks.

> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Mon Aug 30 18:22:36 2010
> @@ -16,6 +16,7 @@
> #include "llvm/IntrinsicInst.h"
> #include "llvm/LLVMContext.h"
> #include "llvm/Pass.h"
> +#include "llvm/Analysis/ConstantFolding.h"

This isn't needed.

> @@ -325,9 +326,10 @@
>       } else if (LVI) {
>         Constant *CI = LVI->getConstantOnEdge(InVal,
>                                               PN->getIncomingBlock(i), BB);
> +        // LVI returns null is no value could be determined.
> +        if (!CI) continue;
> +        ConstantInt *CInt = dyn_cast<ConstantInt>(CI);
> +        Result.push_back(std::make_pair(CInt, PN->getIncomingBlock(i)));

I'm pretty sure that this is not safe/correct if CI is a constantexpr.

> @@ -400,12 +398,19 @@
>     // AND or OR of a value with itself is that value.
>     ConstantInt *CI = dyn_cast<ConstantInt>(BO->getOperand(1));
>     if (CI && (BO->getOpcode() == Instruction::And ||
> +        BO->getOpcode() == Instruction::Or)) {
>       SmallVector<std::pair<ConstantInt*, BasicBlock*>, 8> LHSVals;
>       ComputeValueKnownInPredecessors(BO->getOperand(0), BB, LHSVals);
> +      for (unsigned i = 0, e = LHSVals.size(); i != e; ++i)
> +        if (LHSVals[i].first == 0) {
> +          ConstantInt *Zero =
> +            cast<ConstantInt>(ConstantInt::get(BO->getType(), 0));
> +          Result.push_back(std::make_pair(Zero, LHSVals[i].second));

This isn't correct for 'or'.  undef | x -> -1.

It would be best to turn "null" into an UndefValue, then pass *that* into ConstantExpr::get, then decode that back into undef/ConstantInt/other.
> @@ -472,20 +477,18 @@
> 
>       // Try to find a constant value for the LHS of an equality comparison,
>       // and evaluate it statically if we can.
> +      if (Constant *CmpConst = dyn_cast<Constant>(Cmp->getOperand(1))) {
>         SmallVector<std::pair<ConstantInt*, BasicBlock*>, 8> LHSVals;
>         ComputeValueKnownInPredecessors(I->getOperand(0), BB, LHSVals);
> 
>         for (unsigned i = 0, e = LHSVals.size(); i != e; ++i) {
> +          if (LHSVals[i].first == 0)
> +            Result.push_back(std::make_pair((ConstantInt*)0,
> +                                            LHSVals[i].second));
> +          else if (Constant *Folded = ConstantExpr::getCompare(
> +                               Cmp->getPredicate(), LHSVals[i].first, CmpConst))
> +            Result.push_back(std::make_pair(cast<ConstantInt>(Folded),
> +                                            LHSVals[i].second));

This will abort if ConstantExpr::getCompare returns a ConstantExpr, which it can.

I'm still waiting for testcases.

-Chris