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

LAA: add missed swap when inverting src, sink #122254

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

artagnon
Copy link
Contributor

@artagnon artagnon commented Jan 9, 2025

When inverting source and sink on a negative induction step, the types of the source and sink should also be swapped. This fixes a bug in the code that follows, that computes properties based on these types. With 234cc40 ([LAA] Limit no-overlap check to at least one loop-invariant accesses.), that code is guarded by a loop-invariant condition: however, the commit did not add any new tests exercising the guarded code, and hence the bugfix in this patch requires additional tests to exercise that guarded codepath.

@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

We miss swapping the types of the source and sink when the source and sink are inverted, leaving a footgun behind. Fix this.


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

1 Files Affected:

  • (modified) llvm/lib/Analysis/LoopAccessAnalysis.cpp (+1)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 2c75d5625cb66d..de0f81c0b6a4bf 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1921,6 +1921,7 @@ MemoryDepChecker::getDependenceDistanceStrideAndSize(
   if (StrideAPtr && *StrideAPtr < 0) {
     std::swap(Src, Sink);
     std::swap(AInst, BInst);
+    std::swap(ATy, BTy);
     std::swap(StrideAPtr, StrideBPtr);
   }
 

Copy link
Contributor

@david-arm david-arm left a comment

Choose a reason for hiding this comment

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

LGTM! Good spot. :)

When inverting source and sink on a negative induction step, the types
of the source and sink should also be swapped. This fixes a bug in the
code that follows, that computes properties based on these types. With
234cc40 ([LAA] Limit no-overlap check to at least one loop-invariant
accesses.), that code is guarded by a loop-invariant condition: however,
the commit did not add any new tests exercising the guarded code, and
hence the bugfix in this patch requires additional tests to exercise
that guarded codepath.
@artagnon artagnon force-pushed the laa-stride-typesz-swap branch from 5434156 to 102fb50 Compare January 9, 2025 13:48
@artagnon artagnon changed the title LAA: add missed swap when inverting src, sink (NFC) LAA: add missed swap when inverting src, sink Jan 9, 2025
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.

4 participants