[llvm] r275339 - [SCCP] Have the logic for replacing insts with constant in a single place.

[Rafael Espíndola via llvm-commits]

For some reason this change has a big compile time impact. Could you
revert it for now?

Cheers,
Rafael


On 13 July 2016 at 19:20, Davide Italiano via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: davide
> Date: Wed Jul 13 18:20:04 2016
> New Revision: 275339
>
> URL: http://llvm.org/viewvc/llvm-project?rev=275339&view=rev
> Log:
> [SCCP] Have the logic for replacing insts with constant in a single place.
>
> The code was pretty much copy-pasted between SCCP and IPSCCP. The situation
> became clearly worse after I introduced the support for folding structs in
> SCCP.  This commit is NFC as we currently (still) skip the replacement
> step in IPSCCP, but I'll change this soon.
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SCCP.cpp?rev=275339&r1=275338&r2=275339&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SCCP.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SCCP.cpp Wed Jul 13 18:20:04 2016
> @@ -1510,6 +1510,43 @@ bool SCCPSolver::ResolvedUndefsIn(Functi
>    return false;
>  }
>
> +static bool tryToReplaceInstWithConstant(SCCPSolver Solver, Instruction *Inst,
> +                                         bool shouldEraseFromParent) {
> +  Constant *Const = nullptr;
> +  if (Inst->getType()->isStructTy()) {
> +    std::vector<LatticeVal> IVs = Solver.getStructLatticeValueFor(Inst);
> +    if (std::any_of(IVs.begin(), IVs.end(),
> +                    [](LatticeVal &LV) { return LV.isOverdefined(); }))
> +      return false;
> +    std::vector<Constant *> ConstVals;
> +    StructType *ST = dyn_cast<StructType>(Inst->getType());
> +    for (unsigned i = 0, e = ST->getNumElements(); i != e; ++i) {
> +      LatticeVal V = IVs[i];
> +      ConstVals.push_back(V.isConstant()
> +                              ? V.getConstant()
> +                              : UndefValue::get(ST->getElementType(i)));
> +    }
> +    Const = ConstantStruct::get(ST, ConstVals);
> +  } else {
> +    LatticeVal IV = Solver.getLatticeValueFor(Inst);
> +    if (IV.isOverdefined())
> +      return false;
> +
> +    Const =
> +        IV.isConstant() ? IV.getConstant() : UndefValue::get(Inst->getType());
> +  }
> +  assert(Const && "Constant is nullptr here!");
> +  DEBUG(dbgs() << "  Constant: " << *Const << " = " << *Inst << '\n');
> +
> +  // Replaces all of the uses of a variable with uses of the constant.
> +  Inst->replaceAllUsesWith(Const);
> +
> +  // Delete the instruction.
> +  if (shouldEraseFromParent)
> +    Inst->eraseFromParent();
> +  return true;
> +}
> +
>  // runSCCP() - Run the Sparse Conditional Constant Propagation algorithm,
>  // and return true if the function was modified.
>  //
> @@ -1558,41 +1595,12 @@ static bool runSCCP(Function &F, const D
>        if (Inst->getType()->isVoidTy() || isa<TerminatorInst>(Inst))
>          continue;
>
> -      Constant *Const = nullptr;
> -      if (Inst->getType()->isStructTy()) {
> -        std::vector<LatticeVal> IVs = Solver.getStructLatticeValueFor(Inst);
> -        if (std::any_of(IVs.begin(), IVs.end(),
> -                        [](LatticeVal &LV) { return LV.isOverdefined(); }))
> -          continue;
> -        std::vector<Constant *> ConstVals;
> -        StructType *ST = dyn_cast<StructType>(Inst->getType());
> -        for (unsigned i = 0, e = ST->getNumElements(); i != e; ++i) {
> -          LatticeVal V = IVs[i];
> -          ConstVals.push_back(V.isConstant()
> -                                  ? V.getConstant()
> -                                  : UndefValue::get(ST->getElementType(i)));
> -        }
> -        Const = ConstantStruct::get(ST, ConstVals);
> -      } else {
> -        LatticeVal IV = Solver.getLatticeValueFor(Inst);
> -        if (IV.isOverdefined())
> -          continue;
> -
> -        Const = IV.isConstant() ? IV.getConstant()
> -                                : UndefValue::get(Inst->getType());
> +      if (tryToReplaceInstWithConstant(Solver, Inst,
> +                                       true /* shouldEraseFromParent */)) {
> +        // Hey, we just changed something!
> +        MadeChanges = true;
> +        ++NumInstRemoved;
>        }
> -      assert(Const && "Constant is nullptr here!");
> -      DEBUG(dbgs() << "  Constant: " << *Const << " = " << *Inst << '\n');
> -
> -      // Replaces all of the uses of a variable with uses of the constant.
> -      Inst->replaceAllUsesWith(Const);
> -
> -      // Delete the instruction.
> -      Inst->eraseFromParent();
> -
> -      // Hey, we just changed something!
> -      MadeChanges = true;
> -      ++NumInstRemoved;
>      }
>    }
>
> @@ -1795,25 +1803,14 @@ static bool runIPSCCP(Module &M, const D
>          // TODO: Could use getStructLatticeValueFor to find out if the entire
>          // result is a constant and replace it entirely if so.
>
> -        LatticeVal IV = Solver.getLatticeValueFor(Inst);
> -        if (IV.isOverdefined())
> -          continue;
> -
> -        Constant *Const = IV.isConstant()
> -          ? IV.getConstant() : UndefValue::get(Inst->getType());
> -        DEBUG(dbgs() << "  Constant: " << *Const << " = " << *Inst << '\n');
> -
> -        // Replaces all of the uses of a variable with uses of the
> -        // constant.
> -        Inst->replaceAllUsesWith(Const);
> -
> -        // Delete the instruction.
> -        if (!isa<CallInst>(Inst) && !isa<TerminatorInst>(Inst))
> -          Inst->eraseFromParent();
> -
> -        // Hey, we just changed something!
> -        MadeChanges = true;
> -        ++IPNumInstRemoved;
> +        if (tryToReplaceInstWithConstant(
> +                Solver, Inst,
> +                !isa<CallInst>(Inst) &&
> +                    !isa<TerminatorInst>(Inst) /* shouldEraseFromParent */)) {
> +          // Hey, we just changed something!
> +          MadeChanges = true;
> +          ++IPNumInstRemoved;
> +        }
>        }
>      }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits