[llvm] 22a5cde - [SCEV] Separate out constant folding in mul expr creation

[David Blaikie via llvm-commits]

On Sun, Oct 25, 2020 at 10:47 AM Nikita Popov via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Nikita Popov
> Date: 2020-10-25T18:46:50+01:00
> New Revision: 22a5cde541c3306e53279fd02c6d60ae6178ef10
>
> URL: https://github.com/llvm/llvm-project/commit/22a5cde541c3306e53279fd02c6d60ae6178ef10
> DIFF: https://github.com/llvm/llvm-project/commit/22a5cde541c3306e53279fd02c6d60ae6178ef10.diff
>
> LOG: [SCEV] Separate out constant folding in mul expr creation
>
> Separate out the code handling constant folding into a separate
> block, that is independent of other folds that need a constant
> first operand. Also make some minor adjustments to make the
> constant folding look nearly identical to the same code in
> getAddExpr().
>
> The only reason this change is not strictly NFC is that the
> C1*(C2+V) fold is moved below the constant folding, which means
> that it now also applies to C1*C2*(C3+V), as it should.

If it's not NFC, then this new functionality can and should be tested, I think?

>
> Added:
>
>
> Modified:
>     llvm/lib/Analysis/ScalarEvolution.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
> index e4ed367a6962..92f9949188a1 100644
> --- a/llvm/lib/Analysis/ScalarEvolution.cpp
> +++ b/llvm/lib/Analysis/ScalarEvolution.cpp
> @@ -2700,8 +2700,32 @@ const SCEV *ScalarEvolution::getMulExpr(SmallVectorImpl<const SCEV *> &Ops,
>    // If there are any constants, fold them together.
>    unsigned Idx = 0;
>    if (const SCEVConstant *LHSC = dyn_cast<SCEVConstant>(Ops[0])) {
> +    ++Idx;
> +    assert(Idx < Ops.size());
> +    while (const SCEVConstant *RHSC = dyn_cast<SCEVConstant>(Ops[Idx])) {
> +      // We found two constants, fold them together!
> +      Ops[0] = getConstant(LHSC->getAPInt() * RHSC->getAPInt());
> +      if (Ops.size() == 2) return Ops[0];
> +      Ops.erase(Ops.begin()+1);  // Erase the folded element
> +      LHSC = cast<SCEVConstant>(Ops[0]);
> +    }
> +
> +    // If we have a multiply of zero, it will always be zero.
> +    if (LHSC->getValue()->isZero())
> +      return LHSC;
> +
> +    // If we are left with a constant one being multiplied, strip it off.
> +    if (LHSC->getValue()->isOne()) {
> +      Ops.erase(Ops.begin());
> +      --Idx;
> +    }
> +
> +    if (Ops.size() == 1)
> +      return Ops[0];
> +  }
>
> -    if (Ops.size() == 2)
> +  if (const SCEVConstant *LHSC = dyn_cast<SCEVConstant>(Ops[0])) {
> +    if (Ops.size() == 2) {
>        // C1*(C2+V) -> C1*C2 + C1*V
>        if (const SCEVAddExpr *Add = dyn_cast<SCEVAddExpr>(Ops[1]))
>          // If any of Add's ops are Adds or Muls with a constant, apply this
> @@ -2717,28 +2741,9 @@ const SCEV *ScalarEvolution::getMulExpr(SmallVectorImpl<const SCEV *> &Ops,
>                                         SCEV::FlagAnyWrap, Depth + 1),
>                              SCEV::FlagAnyWrap, Depth + 1);
>
> -    ++Idx;
> -    while (const SCEVConstant *RHSC = dyn_cast<SCEVConstant>(Ops[Idx])) {
> -      // We found two constants, fold them together!
> -      ConstantInt *Fold =
> -          ConstantInt::get(getContext(), LHSC->getAPInt() * RHSC->getAPInt());
> -      Ops[0] = getConstant(Fold);
> -      Ops.erase(Ops.begin()+1);  // Erase the folded element
> -      if (Ops.size() == 1) return Ops[0];
> -      LHSC = cast<SCEVConstant>(Ops[0]);
> -    }
> -
> -    // If we are left with a constant one being multiplied, strip it off.
> -    if (cast<SCEVConstant>(Ops[0])->getValue()->isOne()) {
> -      Ops.erase(Ops.begin());
> -      --Idx;
> -    } else if (cast<SCEVConstant>(Ops[0])->getValue()->isZero()) {
> -      // If we have a multiply of zero, it will always be zero.
> -      return Ops[0];
> -    } else if (Ops[0]->isAllOnesValue()) {
> -      // If we have a mul by -1 of an add, try distributing the -1 among the
> -      // add operands.
> -      if (Ops.size() == 2) {
> +      if (Ops[0]->isAllOnesValue()) {
> +        // If we have a mul by -1 of an add, try distributing the -1 among the
> +        // add operands.
>          if (const SCEVAddExpr *Add = dyn_cast<SCEVAddExpr>(Ops[1])) {
>            SmallVector<const SCEV *, 4> NewOps;
>            bool AnyFolded = false;
> @@ -2762,9 +2767,6 @@ const SCEV *ScalarEvolution::getMulExpr(SmallVectorImpl<const SCEV *> &Ops,
>          }
>        }
>      }
> -
> -    if (Ops.size() == 1)
> -      return Ops[0];
>    }
>
>    // Skip over the add expression until we get to a multiply.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits