[llvm] r374535 - [SCEV] Add stricter verification option.

[Philip Reames via llvm-commits]

I may be missing the obvious, but why is a symbolic expression which 
*may* be zero a violation here?  It would seem to be a missed 
canonicalization, nothing more.

I agree that a SCEV which is known (via isKnownPredicate?) not to be 
zero is a bug.

Philip

On 10/11/2019 4:46 AM, Florian Hahn via llvm-commits wrote:
> Author: fhahn
> Date: Fri Oct 11 04:46:40 2019
> New Revision: 374535
>
> URL: http://llvm.org/viewvc/llvm-project?rev=374535&view=rev
> Log:
> [SCEV] Add stricter verification option.
>
> Currently -verify-scev only fails if there is a constant difference
> between two BE counts. This misses a lot of cases.
>
> This patch adds a -verify-scev-strict options, which fails for any
> non-zero differences, if used together with -verify-scev.
>
> With the stricter checking, some unit tests fail because
> of mis-matches, especially around IndVarSimplify.
>
> If there is no reason I am missing for just checking constant deltas, I
> am planning on looking into the various failures.
>
> Reviewers: efriedma, sanjoy.google, reames, atrick
>
> Reviewed By: sanjoy.google
>
> Differential Revision: https://reviews.llvm.org/D68592
>
> Modified:
>      llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>
> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=374535&r1=374534&r2=374535&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Fri Oct 11 04:46:40 2019
> @@ -158,6 +158,9 @@ MaxBruteForceIterations("scalar-evolutio
>   static cl::opt<bool> VerifySCEV(
>       "verify-scev", cl::Hidden,
>       cl::desc("Verify ScalarEvolution's backedge taken counts (slow)"));
> +static cl::opt<bool> VerifySCEVStrict(
> +    "verify-scev-strict", cl::Hidden,
> +    cl::desc("Enable stricter verification with -verify-scev is passed"));
>   static cl::opt<bool>
>       VerifySCEVMap("verify-scev-maps", cl::Hidden,
>                     cl::desc("Verify no dangling value in ScalarEvolution's "
> @@ -11922,14 +11925,14 @@ void ScalarEvolution::verify() const {
>                SE.getTypeSizeInBits(NewBECount->getType()))
>         CurBECount = SE2.getZeroExtendExpr(CurBECount, NewBECount->getType());
>   
> -    auto *ConstantDelta =
> -        dyn_cast<SCEVConstant>(SE2.getMinusSCEV(CurBECount, NewBECount));
> +    const SCEV *Delta = SE2.getMinusSCEV(CurBECount, NewBECount);
>   
> -    if (ConstantDelta && ConstantDelta->getAPInt() != 0) {
> -      dbgs() << "Trip Count Changed!\n";
> +    // Unless VerifySCEVStrict is set, we only compare constant deltas.
> +    if ((VerifySCEVStrict || isa<SCEVConstant>(Delta)) && !Delta->isZero()) {
> +      dbgs() << "Trip Count for " << *L << " Changed!\n";
>         dbgs() << "Old: " << *CurBECount << "\n";
>         dbgs() << "New: " << *NewBECount << "\n";
> -      dbgs() << "Delta: " << *ConstantDelta << "\n";
> +      dbgs() << "Delta: " << *Delta << "\n";
>         std::abort();
>       }
>     }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits