Skip to content

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Sep 15, 2025

When adding a new predicate to a union, we currently do a bidirectional implication for all the contained predicates. This means that the number of implication checks is quadratic in the number of total predicates (if they don't end up being eliminated).

Fix this by not checking for implication if the number of predicates grows too large. The expectation is that if there is a large number of predicates, we should be discarding them later anyway, as expanding them would be too expensive.

(Alternatively we could also try to prevent the addition of many predicates in the first place, but I figured this is better as it's centralized in one place.)

Fixes #156114.

When adding a new predicate to a union, we currently do a bidirectional
implication for all the contained predicates. This means that the
number of implication checks is quadratic in the number of total
predicates (if they don't end up being eliminated).

Fix this by not checking for implication if the number of predicates
grows too large. The expectation is that if there is a large number
of predicates, we should be discarding them later anyway, as expanding
them would be too expensive.

(Alternatively we could also try to prevent the addition of many
predicates in the first place, but I figured this is better as it's
centralized in one place.)

Fixes llvm#156114.
@nikic nikic requested a review from fhahn September 15, 2025 15:06
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Sep 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 15, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

Changes

When adding a new predicate to a union, we currently do a bidirectional implication for all the contained predicates. This means that the number of implication checks is quadratic in the number of total predicates (if they don't end up being eliminated).

Fix this by not checking for implication if the number of predicates grows too large. The expectation is that if there is a large number of predicates, we should be discarding them later anyway, as expanding them would be too expensive.

(Alternatively we could also try to prevent the addition of many predicates in the first place, but I figured this is better as it's centralized in one place.)

Fixes #156114.


Full diff: https://github.com/llvm/llvm-project/pull/158652.diff

1 Files Affected:

  • (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+7-2)
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index a1703a270952e..9698ed80730ee 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -15176,15 +15176,20 @@ void SCEVUnionPredicate::add(const SCEVPredicate *N, ScalarEvolution &SE) {
     return;
   }
 
+  // Implication checks are quadratic in the number of predicates. Stop doing
+  // them if there are many predicates, as they should be too expensive to use
+  // anyway at that point.
+  bool CheckImplies = Preds.size() < 16;
+
   // Only add predicate if it is not already implied by this union predicate.
-  if (implies(N, SE))
+  if (CheckImplies && implies(N, SE))
     return;
 
   // Build a new vector containing the current predicates, except the ones that
   // are implied by the new predicate N.
   SmallVector<const SCEVPredicate *> PrunedPreds;
   for (auto *P : Preds) {
-    if (N->implies(P, SE))
+    if (CheckImplies && N->implies(P, SE))
       continue;
     PrunedPreds.push_back(P);
   }

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@nikic nikic merged commit 7af659d into llvm:main Sep 16, 2025
11 of 12 checks passed
@nikic nikic deleted the scev-pred-cutoff branch September 16, 2025 07:24
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Sep 23, 2025
…8652)

When adding a new predicate to a union, we currently do a bidirectional
implication for all the contained predicates. This means that the number
of implication checks is quadratic in the number of total predicates (if
they don't end up being eliminated).

Fix this by not checking for implication if the number of predicates
grows too large. The expectation is that if there is a large number of
predicates, we should be discarding them later anyway, as expanding them
would be too expensive.

Fixes llvm#156114.

(cherry picked from commit 7af659d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stall inside LoopLoadEliminationPass
3 participants