[SCEV] Support addrec in right hand side in howManyLessThans #92560

mrdaybird · 2024-05-17T14:45:10Z

Fixes #92554 (std::reverse will auto-vectorize now)

When calculating number of times a exit condition containing a comparison is executed, we mostly assume that RHS of comparison should be loop invariant, but it may be another add-recurrence.

~~In that case, we can try the computation with LHS = LHS - RHS and RHS = 0.~~ (It is not valid unless proven that it doesn't wrap)

Edit:
We can calculate back edge count for loop structure like:

left = left_start
right = right_start
while(left < right){
  // ...do something...
  left += s1; // the stride of left is s1 (> 0)
  right -= s2; // the stride of right is -s2 (s2 > 0)
}
// left and right converge somewhere in the middle of their start values

We can calculate the backedge-count as ceil((End - left_start) /u (s1- (-s2)) where, End = max(left_start, right_start).

Alive2: https://alive2.llvm.org/ce/z/ggxx58

github-actions · 2024-05-17T14:45:27Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-05-17T14:45:56Z

@llvm/pr-subscribers-llvm-analysis

Author: vaibhav (mrdaybird)

Changes

Fixes #92554 (std::reverse will auto-vectorize now)

When calculating number of times a exit condition containing a comparison is executed, we mostly assume that RHS of comparison should be loop invariant, but it may be another add-recurrence.

In that case, we can try the computation with LHS = LHS - RHS and RHS = 0.

For now, I have only changed howManyLessThans, so it only affects X < Y type of conditions.
Other conditions will also need to be modified.
@fhahn

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

1 Files Affected:

(modified) llvm/lib/Analysis/ScalarEvolution.cpp (+9-5)

diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 515b9d0744f6e..9bf5bf80b4570 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -12941,12 +12941,16 @@ ScalarEvolution::howManyLessThans(const SCEV *LHS, const SCEV *RHS,
       return RHS;
   }
 
-  // When the RHS is not invariant, we do not know the end bound of the loop and
-  // cannot calculate the ExactBECount needed by ExitLimit. However, we can
-  // calculate the MaxBECount, given the start, stride and max value for the end
-  // bound of the loop (RHS), and the fact that IV does not overflow (which is
-  // checked above).
   if (!isLoopInvariant(RHS, L)) {
+    // If RHS is an add recurrence, try again with lhs=lhs-rhs and rhs=0
+    if(auto RHSAddRec = dyn_cast<SCEVAddRecExpr>(RHS)){
+       return howManyLessThans(getMinusSCEV(IV, RHSAddRec), 
+        getZero(IV->getType()), L, true, ControlsOnlyExit, AllowPredicates);
+    }
+    // If we cannot calculate ExactBECount, we can calculate the MaxBECount, 
+    // given the start, stride and max value for the end bound of the 
+    // loop (RHS), and the fact that IV does not overflow (which is
+    // checked above).
     const SCEV *MaxBECount = computeMaxBECountForLT(
         Start, Stride, RHS, getTypeSizeInBits(LHS->getType()), IsSigned);
     return ExitLimit(getCouldNotCompute() /* ExactNotTaken */, MaxBECount,

mrdaybird · 2024-05-17T14:57:12Z

I had a few queries:

What is the need of howManyGreaterThans, can we not reuse howManyLessThans to do the same computations?
The comment in line 9198 mentions that RHS cannot include boundary value. Is this still true if the RHS is an add-recurrence? For example, consider that LHS is a increasing recurrence starting from zero and RHS is decreasing recurrence starting from N>0.

@nikic @fhahn

efriedma-quic · 2024-05-17T16:50:30Z

What is the need of howManyGreaterThans, can we not reuse howManyLessThans to do the same computations?

They probably should be merged... but it's not trivial. If you just blindly NOT both sides, the result would be correct, but probably less accurate. Otherwise, you need to add a bunch of conditionals to track whether you're supposed to be incrementing or decrementing.

The comment in line 9198 mentions that RHS cannot include boundary value. Is this still true if the RHS is an add-recurrence?

In general, if the RHS is loop-variant, the LHS can potentially wrap even if the loop isn't infinite. I don't think restricting to add-recurrences on the RHS changes that.

llvm/lib/Analysis/ScalarEvolution.cpp

mrdaybird · 2024-05-18T13:00:11Z

@efriedma-quic Thanks for answering the questions and reviewing my code!

I tried another approach, with the idea that if we can show that Start of RHS >= Start of LHS, and stride of LHS is positive and stride of RHS is negative, then the variables do not overflow(atleast before the last iteration) thus the exact backedge-count is computable(and the loop is finite).
So, I tried something like this, in place of previous code:

 if (!isLoopInvariant(RHS, L)) {
    // If RHS is an add recurrence, try again with lhs=lhs-rhs and rhs=0
    if(auto RHSAddRec = dyn_cast<SCEVAddRecExpr>(RHS)){
      auto RHSStart = RHSAddRec->getStart();
      auto RHSStride = RHSAddRec->getStepRecurrence(*this);
      auto RHSStartMinusStride = getMinusSCEV(RHSStart, RHSStride);

      auto CondGT = IsSigned ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
      auto *StartMinusOne = getAddExpr(Start,
                                      getMinusOne(Start->getType() ));
      if(isLoopEntryGuardedByCond(L, CondGT, RHSStartMinusStride, StartMinusOne)){
        // Now, we calculate BECount using RHSStart, RHSStride and Start(lhs) and Stride(lhs) 
      }
    }
....

I tried this on a sample code(CE), but I don't know why but isLoopEntryGuardedByCond is not true, eventhough I think it should be. Any idea, what's wrong? @efriedma-quic

Thanks again for the help!

efriedma-quic · 2024-05-18T18:51:59Z

I don't see anything obviously wrong in your example; probably something simple is going wrong. Are you sure the expressions you're computing match the condition? Maybe try setting a breakpoint on ScalarEvolution::isImpliedCondBalancedTypes and step through.

mrdaybird · 2024-05-19T16:52:29Z

@efriedma-quic I have modified the code, maybe you can have a look at it?
If everything's alright, then I will go ahead and add some tests.

efriedma-quic · 2024-05-20T20:07:37Z

The general approach here seems fine; there are some special-cases you could optimize, but we can leave that for a followup.

Please refactor the copy-pasted code so the relevant codepaths are shared.

github-actions · 2024-05-20T20:13:16Z

✅ With the latest revision this PR passed the C/C++ code formatter.

mrdaybird · 2024-05-20T21:05:55Z

@efriedma-quic I copy-pasted the return logic because otherwise I had to push everything into a else branch(which would be pain to review and git-diff 😅)
Also, fixed the formatting issue.

fhahn · 2024-05-20T21:09:43Z

Please also include some test cases covering the various checks

efriedma-quic · 2024-05-20T21:43:04Z

Maybe you can flatten the control flow using the if (!BECount) { checks?

Otherwise, it's not a big deal; we can always review the diff with whitespace turned off.

hiraditya · 2024-05-20T22:05:01Z

Thanks for putting this patch! You might want to use https://alive2.llvm.org/ce/ to verify transformations. It is quite useful in finding corner cases.

nikic

Can you please add some tests in llvm/test/Analysis/ScalarEvolution?

llvm/lib/Analysis/ScalarEvolution.cpp

mrdaybird · 2024-05-21T12:38:51Z

Alive2 proof: https://alive2.llvm.org/ce/z/sXwLZY (updated) -> checks for validity for all start values as well as all strides(i.e. s1, s2 such that s1>0 and s2<0)
The code in proof is based on this c++ code: https://godbolt.org/z/v38TKKajz

mrdaybird · 2024-05-22T08:24:51Z

@nikic I have added tests and verified the code using alive2. I hope everything alright!

mrdaybird · 2024-05-30T17:46:05Z

I guess I will broaden the scope and add optimisations in a follow up PR.

efriedma-quic · 2024-05-30T20:51:09Z

Thinking about it a bit more, there's one thing the alive2 proof currently isn't testing: other exits from the loop (or infinite loops nested inside the loop). In those cases, even if we prove it's impossible to exit via this exit, we need to ensure that the returned backedge-taken count is greater than the number of times the loop actually iterates.

mrdaybird · 2024-05-31T05:53:31Z

@efriedma-quic I am not sure what exactly needs to be done. The case that you mentioned is true in general(i.e. for any type of loop), therefore shouldn't it be already handled by the previous code. Maybe I am misunderstanding you point here? If so, maybe if you could refer me to the original alive2 proof for computing backedge count (or something similar) it would point me in the right direction!

mrdaybird · 2024-06-01T06:09:25Z

@efriedma-quic From what I understand, if we could not find the exact backedge-count(like for the cases you mentioned) we return SCEVCouldNotCompute, which is considered greater than any computable value.

efriedma-quic · 2024-06-01T21:16:45Z

The case I was thinking of is something like https://alive2.llvm.org/ce/z/kzxOYg . Alive2 thinks it's fine as long as we keep the strides-not-equal check.

And yes, we can always return SCEVCouldNotCompute() if we can't prove there's a safe value to return.

mrdaybird · 2024-06-04T07:05:15Z

@efriedma-quic Interesting case that you mentioned! (good to know that it was not a problem)

hiraditya · 2024-06-05T17:04:02Z

LGTM as i dont have any pending concerns. I'd defer to code owners to +1. Thanks for working on it as this enables vectorization of a class of loops. One suggestion would be to run llvm-testsuite or SPEC and see how many new loops got vectorized. Even without getting performance numbers, just -Rpass=vectorize -Rpass-analysis=vectorize stats will be useful.

efriedma-quic

LGTM. Please don't forget to revise the commit message before you merge.

mrdaybird · 2024-06-12T15:29:58Z

@nikic I have improved the commit message. (You can update it if you want!) I hope everything looks good!

hiraditya

Let's wait until Monday in case @nikic has any comments.

github-actions · 2024-06-25T21:34:18Z

@mrdaybird Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

mrdaybird · 2024-06-26T04:55:41Z

Thanks for help (with my first PR)! @efriedma-quic and @hiraditya! Learned a lot! Hopefully I can contribute more to LLVM.

) Fixes llvm#92554 (std::reverse will auto-vectorize now) When calculating number of times a exit condition containing a comparison is executed, we mostly assume that RHS of comparison should be loop invariant, but it may be another add-recurrence. ~In that case, we can try the computation with `LHS = LHS - RHS` and `RHS = 0`.~ (It is not valid unless proven that it doesn't wrap) **Edit:** We can calculate back edge count for loop structure like: ```cpp left = left_start right = right_start while(left < right){ // ...do something... left += s1; // the stride of left is s1 (> 0) right -= s2; // the stride of right is -s2 (s2 > 0) } // left and right converge somewhere in the middle of their start values ``` We can calculate the backedge-count as ceil((End - left_start) /u (s1- (-s2)) where, End = max(left_start, right_start). **Alive2**: https://alive2.llvm.org/ce/z/ggxx58

mrdaybird requested a review from nikic as a code owner May 17, 2024 14:45

llvmbot added the llvm:analysis label May 17, 2024

mrdaybird marked this pull request as draft May 17, 2024 14:58

hiraditya self-requested a review May 17, 2024 15:26

efriedma-quic reviewed May 17, 2024

View reviewed changes

llvm/lib/Analysis/ScalarEvolution.cpp Outdated Show resolved Hide resolved

mrdaybird changed the title ~~Fix exact backedge count algorithm in Scalar-Evolution~~ Handle backedge-count algo for std::reverse like loops May 20, 2024

mrdaybird changed the title ~~Handle backedge-count algo for std::reverse like loops~~ Handle backedge-count logic for std::reverse like loops May 20, 2024

mrdaybird requested a review from efriedma-quic May 20, 2024 06:21

mrdaybird marked this pull request as ready for review May 20, 2024 21:06

mrdaybird marked this pull request as draft May 20, 2024 21:19

hiraditya marked this pull request as ready for review May 20, 2024 22:03

hiraditya added the llvm:SCEV Scalar Evolution label May 20, 2024

nikic changed the title ~~Handle backedge-count logic for std::reverse like loops~~ [SCEV] Handle backedge-count logic for std::reverse like loops May 21, 2024

nikic reviewed May 21, 2024

View reviewed changes

llvm/lib/Analysis/ScalarEvolution.cpp Outdated Show resolved Hide resolved

llvm/lib/Analysis/ScalarEvolution.cpp Outdated Show resolved Hide resolved

mrdaybird requested a review from nikic May 21, 2024 11:13

efriedma-quic approved these changes Jun 9, 2024

View reviewed changes

mrdaybird changed the title ~~[SCEV] Handle backedge-count logic for std::reverse like loops~~ [SCEV] Support comparision with addrec in howManyLessThans Jun 12, 2024

mrdaybird changed the title ~~[SCEV] Support comparision with addrec in howManyLessThans~~ [SCEV] Support addrec in right hand side in howManyLessThans Jun 12, 2024

hiraditya approved these changes Jun 21, 2024

View reviewed changes

mrdaybird added 9 commits June 25, 2024 08:55

Update howManyLessThans

2861ab8

Update howManyLessThans

60c1744

Add negative rhs stride condition

2d78f1f

Fix formatting

1b4945b

Add tests

778a7dd

Add condition for RHS to be in the same loop, fix formatting

35afcbb

Update comment formatting

3394ae0

Check for RHSAddRec nowrap flags and add test

83469ae

Add test and fix last commit

2d49c3b

hiraditya force-pushed the std-reverse branch from da8221a to 2d49c3b Compare June 25, 2024 15:55

hiraditya merged commit 7e59b20 into llvm:main Jun 25, 2024
7 checks passed

mrdaybird mentioned this pull request Jul 5, 2024

[libc++] Vectorize all the algorithms #84663

Open

25 tasks

mrdaybird deleted the std-reverse branch July 7, 2024 18:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SCEV] Support addrec in right hand side in howManyLessThans #92560

[SCEV] Support addrec in right hand side in howManyLessThans #92560

mrdaybird commented May 17, 2024 •

edited

Loading

github-actions bot commented May 17, 2024

llvmbot commented May 17, 2024

mrdaybird commented May 17, 2024 •

edited

Loading

efriedma-quic commented May 17, 2024

mrdaybird commented May 18, 2024

efriedma-quic commented May 18, 2024

mrdaybird commented May 19, 2024

efriedma-quic commented May 20, 2024

github-actions bot commented May 20, 2024 •

edited

Loading

mrdaybird commented May 20, 2024 •

edited

Loading

fhahn commented May 20, 2024

efriedma-quic commented May 20, 2024

hiraditya commented May 20, 2024

nikic left a comment

mrdaybird commented May 21, 2024 •

edited

Loading

mrdaybird commented May 22, 2024

mrdaybird commented May 30, 2024

efriedma-quic commented May 30, 2024

mrdaybird commented May 31, 2024

mrdaybird commented Jun 1, 2024

efriedma-quic commented Jun 1, 2024

mrdaybird commented Jun 4, 2024

hiraditya commented Jun 5, 2024

efriedma-quic left a comment

mrdaybird commented Jun 12, 2024

hiraditya left a comment

github-actions bot commented Jun 25, 2024

mrdaybird commented Jun 26, 2024

[SCEV] Support addrec in right hand side in howManyLessThans #92560

[SCEV] Support addrec in right hand side in howManyLessThans #92560

Conversation

mrdaybird commented May 17, 2024 • edited Loading

github-actions bot commented May 17, 2024

llvmbot commented May 17, 2024

mrdaybird commented May 17, 2024 • edited Loading

efriedma-quic commented May 17, 2024

mrdaybird commented May 18, 2024

efriedma-quic commented May 18, 2024

mrdaybird commented May 19, 2024

efriedma-quic commented May 20, 2024

github-actions bot commented May 20, 2024 • edited Loading

mrdaybird commented May 20, 2024 • edited Loading

fhahn commented May 20, 2024

efriedma-quic commented May 20, 2024

hiraditya commented May 20, 2024

nikic left a comment

Choose a reason for hiding this comment

mrdaybird commented May 21, 2024 • edited Loading

mrdaybird commented May 22, 2024

mrdaybird commented May 30, 2024

efriedma-quic commented May 30, 2024

mrdaybird commented May 31, 2024

mrdaybird commented Jun 1, 2024

efriedma-quic commented Jun 1, 2024

mrdaybird commented Jun 4, 2024

hiraditya commented Jun 5, 2024

efriedma-quic left a comment

Choose a reason for hiding this comment

mrdaybird commented Jun 12, 2024

hiraditya left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 25, 2024

mrdaybird commented Jun 26, 2024

mrdaybird commented May 17, 2024 •

edited

Loading

mrdaybird commented May 17, 2024 •

edited

Loading

github-actions bot commented May 20, 2024 •

edited

Loading

mrdaybird commented May 20, 2024 •

edited

Loading

mrdaybird commented May 21, 2024 •

edited

Loading