Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SCEV] Use loop guards when checking that RHS >= Start #75039

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Dec 11, 2023

Loop guards tend to provide better results when it comes to reasoning about ranges than isLoopEntryGuardedByCond(). See the test change for the motivating case.

I have retained both the loop guard check and the implied cond based check for now, though the latter only seems to impact a single test and only via side effects (nowrap flag calculation) at that.

Loop guards tend to provide better results when it comes to reasoning
about ranges than isLoopEntryGuardedByCond(). See the test change
for the motivating case.

I had retained both the loop guard check and the implied cond based
check for now, though the latter only seems to impact a single test
and only via side effects (nowrap flag calculation) at that.
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2023

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

Changes

Loop guards tend to provide better results when it comes to reasoning about ranges than isLoopEntryGuardedByCond(). See the test change for the motivating case.

I have retained both the loop guard check and the implied cond based check for now, though the latter only seems to impact a single test and only via side effects (nowrap flag calculation) at that.


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+5-1)
  • (modified) llvm/test/Analysis/ScalarEvolution/trip-count.ll (+3-3)
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 451ae73dbd601c..580fe112fcd7bd 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -12910,7 +12910,11 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
   if (!BECount) {
     auto canProveRHSGreaterThanEqualStart = [&]() {
       auto CondGE = IsSigned ? ICmpInst::ICMP_SGE : ICmpInst::ICMP_UGE;
-      if (isLoopEntryGuardedByCond(L, CondGE, OrigRHS, OrigStart))
+      const SCEV *GuardedRHS = applyLoopGuards(OrigRHS, L);
+      const SCEV *GuardedStart = applyLoopGuards(OrigStart, L);
+
+      if (isLoopEntryGuardedByCond(L, CondGE, OrigRHS, OrigStart) ||
+          isKnownPredicate(CondGE, GuardedRHS, GuardedStart))
         return true;
 
       // (RHS > Start - 1) implies RHS >= Start.
diff --git a/llvm/test/Analysis/ScalarEvolution/trip-count.ll b/llvm/test/Analysis/ScalarEvolution/trip-count.ll
index 22e49ebdbf4dbf..36b42c62dd3945 100644
--- a/llvm/test/Analysis/ScalarEvolution/trip-count.ll
+++ b/llvm/test/Analysis/ScalarEvolution/trip-count.ll
@@ -127,10 +127,10 @@ leave:
 define void @non_zero_from_loop_guard(i16 %n) {
 ; CHECK-LABEL: 'non_zero_from_loop_guard'
 ; CHECK-NEXT:  Determining loop execution counts for: @non_zero_from_loop_guard
-; CHECK-NEXT:  Loop %loop: backedge-taken count is (-1 + (1 umax (%n /u 2)))<nsw>
+; CHECK-NEXT:  Loop %loop: backedge-taken count is (-1 + (%n /u 2))<nsw>
 ; CHECK-NEXT:  Loop %loop: constant max backedge-taken count is 32766
-; CHECK-NEXT:  Loop %loop: symbolic max backedge-taken count is (-1 + (1 umax (%n /u 2)))<nsw>
-; CHECK-NEXT:  Loop %loop: Predicated backedge-taken count is (-1 + (1 umax (%n /u 2)))<nsw>
+; CHECK-NEXT:  Loop %loop: symbolic max backedge-taken count is (-1 + (%n /u 2))<nsw>
+; CHECK-NEXT:  Loop %loop: Predicated backedge-taken count is (-1 + (%n /u 2))<nsw>
 ; CHECK-NEXT:   Predicates:
 ; CHECK-NEXT:  Loop %loop: Trip multiple is 1
 ;

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 assuming the compile-time impact isn't too bad!

Using loop guards throughout the various howMany* would probably be beneficial in general, but that would probably need some sort of caching of the collected conds to keep compile-time impact low

@nikic
Copy link
Contributor Author

nikic commented Dec 11, 2023

LGTM, thanks assuming the compile-time impact isn't too bad!

Using loop guards throughout the various howMany* would probably be beneficial in general, but that would probably need some sort of caching of the collected conds to keep compile-time impact low

It looks like there isn't any significant impact: http://llvm-compile-time-tracker.com/compare.php?from=18959c46e3ced1f7ad12a82e9f30bafe9d1f1733&to=fac70f5663ff8f7e87a019603e0e1879110e9772&stat=instructions:u

I think BECount calculation is generally not very compile-time sensitive (as it's once per loop), so we can likely be fairly liberal with use of loop guards in there.

@nikic nikic merged commit 90d8241 into llvm:main Dec 12, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants