[llvm] 2bb3515 - [SCEV] Replace NumTripCountsComputed stat with NumExitCountsComputed
Dmitry Makogon via llvm-commits
llvm-commits at lists.llvm.org
Mon May 22 06:12:00 PDT 2023
Author: Dmitry Makogon
Date: 2023-05-22T20:10:51+07:00
New Revision: 2bb35151524fa35c7840367f1bc8b4966a3d5cf3
URL: https://github.com/llvm/llvm-project/commit/2bb35151524fa35c7840367f1bc8b4966a3d5cf3
DIFF: https://github.com/llvm/llvm-project/commit/2bb35151524fa35c7840367f1bc8b4966a3d5cf3.diff
LOG: [SCEV] Replace NumTripCountsComputed stat with NumExitCountsComputed
This fixes assertion crash in https://github.com/llvm/llvm-project/issues/62380.
In the beginning of ScalarEvolution::getBackedgeTakenInfo
we make sure that BackedgeTakenCounts contains an entry
for the given loop.
Then we call computeBackedgeTakenCount which computes the result,
and in the end we insert it in the map like so:
return BackedgeTakenCounts.find(L)->second = std::move(Result);
So we expect that the entry for L still exists in the cache.
However, it can get deleted. When it has computed the result,
getBackedgeTakenInfo clears all the cached SCEVs that use the AddRecs in the loop.
In the crashing example, getBackedgeTakenInfo first gets called on an inner loop,
and during this call it gets called again on its parent loop.
This recursion happens after the call to computeBackedgeTakenCount.
And it happens so that some SCEV from the BTI of the child loop uses
an AddRec of the parent loop. So when we successfully compute BTI
for the parent loop, we erase already computed result for the child one.
The recursion happens in some debug only code that
updates statistics. The algorithm itself is non-recursive.
Namely the recursive call happens in BackedgeTakenInfo::getExact function
and its return value is only used to compare it against SCEVCouldNotCompute.
As suggested by nikic I replaced the NumTripCountsComputed and NumTripCountsNotComputed
with NumExitCountsComputed and NumExitCountsNotComputed respectively.
They are updated during computations made for single exits. It relieves us of the need
to compute exact exit count for the loop just to update the named
statistic and thus the recursion cannot happen anymore.
Differential Revision: https://reviews.llvm.org/D149251
Added:
Modified:
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/test/Analysis/ScalarEvolution/pr62380.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index aec1e34fd27bc..2ad787053f837 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -135,10 +135,10 @@ using namespace PatternMatch;
#define DEBUG_TYPE "scalar-evolution"
-STATISTIC(NumTripCountsComputed,
- "Number of loops with predictable loop counts");
-STATISTIC(NumTripCountsNotComputed,
- "Number of loops without predictable loop counts");
+STATISTIC(NumExitCountsComputed,
+ "Number of loop exits with predictable exit counts");
+STATISTIC(NumExitCountsNotComputed,
+ "Number of loop exits without predictable exit counts");
STATISTIC(NumBruteForceTripCountsComputed,
"Number of loops with trip counts computed by force");
@@ -8450,23 +8450,6 @@ ScalarEvolution::getBackedgeTakenInfo(const Loop *L) {
// must be cleared in this scope.
BackedgeTakenInfo Result = computeBackedgeTakenCount(L);
- // In product build, there are no usage of statistic.
- (void)NumTripCountsComputed;
- (void)NumTripCountsNotComputed;
-#if LLVM_ENABLE_STATS || !defined(NDEBUG)
- const SCEV *BEExact = Result.getExact(L, this);
- if (BEExact != getCouldNotCompute()) {
- assert(isLoopInvariant(BEExact, L) &&
- isLoopInvariant(Result.getConstantMax(this), L) &&
- "Computed backedge-taken count isn't loop invariant for loop!");
- ++NumTripCountsComputed;
- } else if (Result.getConstantMax(this) == getCouldNotCompute() &&
- isa<PHINode>(L->getHeader()->begin())) {
- // Only count loops that have phi nodes as not being computable.
- ++NumTripCountsNotComputed;
- }
-#endif // LLVM_ENABLE_STATS || !defined(NDEBUG)
-
// Now that we know more about the trip count for this loop, forget any
// existing SCEV values for PHI nodes in this loop since they are only
// conservative estimates made without the benefit of trip count
@@ -8852,7 +8835,9 @@ ScalarEvolution::computeBackedgeTakenCount(const Loop *L,
// 1. For each exit that can be computed, add an entry to ExitCounts.
// CouldComputeBECount is true only if all exits can be computed.
- if (EL.ExactNotTaken == getCouldNotCompute())
+ if (EL.ExactNotTaken != getCouldNotCompute())
+ ++NumExitCountsComputed;
+ else
// We couldn't compute an exact value for this exit, so
// we won't be able to compute an exact value for the loop.
CouldComputeBECount = false;
@@ -8860,9 +8845,11 @@ ScalarEvolution::computeBackedgeTakenCount(const Loop *L,
// Exact always implies symbolic, only check symbolic.
if (EL.SymbolicMaxNotTaken != getCouldNotCompute())
ExitCounts.emplace_back(ExitBB, EL);
- else
+ else {
assert(EL.ExactNotTaken == getCouldNotCompute() &&
"Exact is known but symbolic isn't?");
+ ++NumExitCountsNotComputed;
+ }
// 2. Derive the loop's MaxBECount from each exit's max number of
// non-exiting iterations. Partition the loop exits into two kinds:
diff --git a/llvm/test/Analysis/ScalarEvolution/pr62380.ll b/llvm/test/Analysis/ScalarEvolution/pr62380.ll
index 59e8717a994f0..25acd43143883 100644
--- a/llvm/test/Analysis/ScalarEvolution/pr62380.ll
+++ b/llvm/test/Analysis/ScalarEvolution/pr62380.ll
@@ -1,12 +1,50 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
; RUN: opt -passes='loop(loop-deletion),loop-mssa(loop-predication,licm<allowspeculation>,simple-loop-unswitch<nontrivial>),loop(loop-predication)' -S < %s | FileCheck %s
-; REQUIRES: asserts
-; XFAIL: *
-
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
target triple = "x86_64-unknown-linux-gnu"
define void @test(i32 %arg) {
+; CHECK-LABEL: define void @test
+; CHECK-SAME: (i32 [[ARG:%.*]]) {
+; CHECK-NEXT: bb:
+; CHECK-NEXT: br label [[BB1:%.*]]
+; CHECK: bb1:
+; CHECK-NEXT: br label [[BB2:%.*]]
+; CHECK: bb2:
+; CHECK-NEXT: br i1 false, label [[BB3_PREHEADER:%.*]], label [[BB1]]
+; CHECK: bb3.preheader:
+; CHECK-NEXT: [[LOAD_LE:%.*]] = load i32, ptr null, align 4
+; CHECK-NEXT: br label [[BB3:%.*]]
+; CHECK: bb3.loopexit:
+; CHECK-NEXT: br label [[BB3]]
+; CHECK: bb3:
+; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[ADD:%.*]], [[BB3_LOOPEXIT:%.*]] ], [ 0, [[BB3_PREHEADER]] ]
+; CHECK-NEXT: [[ADD]] = add i32 [[PHI]], 1
+; CHECK-NEXT: [[ICMP:%.*]] = icmp ult i32 [[PHI]], [[LOAD_LE]]
+; CHECK-NEXT: br i1 [[ICMP]], label [[BB5:%.*]], label [[BB4:%.*]]
+; CHECK: bb4:
+; CHECK-NEXT: ret void
+; CHECK: bb5:
+; CHECK-NEXT: [[CALL:%.*]] = call i1 @llvm.experimental.widenable.condition()
+; CHECK-NEXT: br i1 [[CALL]], label [[BB9_PREHEADER:%.*]], label [[BB14:%.*]]
+; CHECK: bb9.preheader:
+; CHECK-NEXT: br label [[BB9:%.*]]
+; CHECK: bb6:
+; CHECK-NEXT: [[ADD7:%.*]] = add i32 [[PHI10:%.*]], 1
+; CHECK-NEXT: [[ICMP8:%.*]] = icmp ugt i32 [[PHI10]], 1
+; CHECK-NEXT: br i1 [[ICMP8]], label [[BB3_LOOPEXIT]], label [[BB9]]
+; CHECK: bb9:
+; CHECK-NEXT: [[PHI10]] = phi i32 [ [[ADD7]], [[BB6:%.*]] ], [ [[PHI]], [[BB9_PREHEADER]] ]
+; CHECK-NEXT: [[ICMP11:%.*]] = icmp ult i32 [[PHI10]], [[ARG]]
+; CHECK-NEXT: [[CALL12:%.*]] = call i1 @llvm.experimental.widenable.condition()
+; CHECK-NEXT: [[AND:%.*]] = and i1 [[ICMP11]], true
+; CHECK-NEXT: br i1 [[AND]], label [[BB6]], label [[BB13:%.*]]
+; CHECK: bb13:
+; CHECK-NEXT: ret void
+; CHECK: bb14:
+; CHECK-NEXT: ret void
+;
bb:
br label %bb1
More information about the llvm-commits
mailing list