[llvm] r317016 - [IndVarSimplify] Extract wrapper around SE-.isLoopInvariantPredicate [NFC]

[Philip Reames via llvm-commits]

FYI - I've gotten a report that this change has introduced a miscompile 
in a stage2 clang build.  I'm about to revert this and the following 
patch due to lack of time to investigate fully.


On 10/31/2017 11:04 AM, Philip Reames via llvm-commits wrote:
> Author: reames
> Date: Tue Oct 31 11:04:57 2017
> New Revision: 317016
>
> URL: http://llvm.org/viewvc/llvm-project?rev=317016&view=rev
> Log:
> [IndVarSimplify] Extract wrapper around SE-.isLoopInvariantPredicate [NFC]
>
> This an intermediate state, the next patch will re-inline the markLoopInvariantPredicate function to reduce code duplication.
>
>
> Modified:
>      llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp?rev=317016&r1=317015&r2=317016&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp Tue Oct 31 11:04:57 2017
> @@ -83,6 +83,11 @@ namespace {
>   
>       bool eliminateOverflowIntrinsic(CallInst *CI);
>       bool eliminateIVUser(Instruction *UseInst, Instruction *IVOperand);
> +    bool isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred,
> +           const SCEV *LHS, const SCEV *RHS, const Loop *L,
> +           const SmallDenseMap<const SCEV*, Value*> &FreeExpansions,
> +           ICmpInst::Predicate &InvariantPred,
> +           Value *&LHSV, Value *& RHSV);
>       bool makeIVComparisonInvariant(ICmpInst *ICmp, Value *IVOperand);
>       void eliminateIVComparison(ICmpInst *ICmp, Value *IVOperand);
>       void simplifyIVRemainder(BinaryOperator *Rem, Value *IVOperand,
> @@ -162,6 +167,26 @@ Value *SimplifyIndvar::foldIVUser(Instru
>     return IVSrc;
>   }
>   
> +bool SimplifyIndvar::isCheapLoopInvariantPredicate(ICmpInst::Predicate Pred,
> +           const SCEV *LHS, const SCEV *RHS, const Loop *L,
> +           const SmallDenseMap<const SCEV*, Value*> &FreeExpansions,
> +           ICmpInst::Predicate &InvariantPred,
> +           Value *&LHSV, Value *& RHSV) {
> +
> +  const SCEV *InvariantLHS, *InvariantRHS;
> +  if (!SE->isLoopInvariantPredicate(Pred, LHS, RHS, L, InvariantPred,
> +                                    InvariantLHS, InvariantRHS))
> +    return false;
> +
> +  // Rewrite the comparison to a loop invariant comparison if it can be done
> +  // cheaply, where cheaply means "we don't need to emit any new
> +  // instructions".
> +  LHSV = FreeExpansions.lookup(InvariantLHS);
> +  RHSV = FreeExpansions.lookup(InvariantRHS);
> +
> +  return (LHSV && RHSV);
> +}
> +
>   bool SimplifyIndvar::makeIVComparisonInvariant(ICmpInst *ICmp,
>                                                  Value *IVOperand) {
>     unsigned IVOperIdx = 0;
> @@ -179,19 +204,9 @@ bool SimplifyIndvar::makeIVComparisonInv
>     const SCEV *S = SE->getSCEVAtScope(ICmp->getOperand(IVOperIdx), ICmpLoop);
>     const SCEV *X = SE->getSCEVAtScope(ICmp->getOperand(1 - IVOperIdx), ICmpLoop);
>   
> -  ICmpInst::Predicate InvariantPredicate;
> -  const SCEV *InvariantLHS, *InvariantRHS;
> -
>     auto *PN = dyn_cast<PHINode>(IVOperand);
>     if (!PN)
>       return false;
> -  if (!SE->isLoopInvariantPredicate(Pred, S, X, L, InvariantPredicate,
> -                                    InvariantLHS, InvariantRHS))
> -    return false;
> -
> -  // Rewrite the comparison to a loop invariant comparison if it can be done
> -  // cheaply, where cheaply means "we don't need to emit any new
> -  // instructions".
>   
>     SmallDenseMap<const SCEV*, Value*> CheapExpansions;
>     CheapExpansions[S] = ICmp->getOperand(IVOperIdx);
> @@ -204,17 +219,18 @@ bool SimplifyIndvar::makeIVComparisonInv
>       const SCEV *IncomingS = SE->getSCEV(Incoming);
>       CheapExpansions[IncomingS] = Incoming;
>     }
> -  Value *NewLHS = CheapExpansions[InvariantLHS];
> -  Value *NewRHS = CheapExpansions[InvariantRHS];
>   
> -  if (!NewLHS || !NewRHS)
> -    // We could not find an existing value to replace either LHS or RHS.
> -    // Generating new instructions has subtler tradeoffs, so avoid doing that
> -    // for now.
> +  ICmpInst::Predicate NewPred;
> +  Value *NewLHS = nullptr, *NewRHS = nullptr;
> +
> +  if (!isCheapLoopInvariantPredicate(Pred, S, X, L, CheapExpansions,
> +                                     NewPred, NewLHS, NewRHS))
>       return false;
> +
> +  assert(NewLHS && NewRHS);
>   
>     DEBUG(dbgs() << "INDVARS: Simplified comparison: " << *ICmp << '\n');
> -  ICmp->setPredicate(InvariantPredicate);
> +  ICmp->setPredicate(NewPred);
>     ICmp->setOperand(0, NewLHS);
>     ICmp->setOperand(1, NewRHS);
>     return true;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits