Skip to content

DependenceAnalysis is not access size aware. #16183

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

Closed
etherzhhb mannequin opened this issue Apr 22, 2013 · 2 comments
Closed

DependenceAnalysis is not access size aware. #16183

etherzhhb mannequin opened this issue Apr 22, 2013 · 2 comments
Labels
bugzilla Issues migrated from bugzilla llvm:analysis

Comments

@etherzhhb
Copy link
Mannequin

etherzhhb mannequin commented Apr 22, 2013

Bugzilla Link 15811
Version trunk
OS All
Attachments The patch to add the fail case.
CC @etherzhhb

Extended Description

Consider the following function:

define i64 @​alias(i32* nocapture %A) nounwind uwtable {
entry:
%arrayidx = getelementptr inbounds i32* %A, i64 1
store i32 2, i32* %arrayidx, align 4
%0 = bitcast i32* %A to i64*
%1 = load i64* %0, align 8
ret i64 %1
}

Ignoring size of the memory location accessed in
store i32 2, i32* %arrayidx, align 4
and
%1 = load i64* %0, align 8,

The addresses pointed by %arrayidx and %0 doesn't alias each other, and hence DependenceAnalysis claims there is not dependencies from the store to the load, and gives the following output:
da analyze - none!
da analyze - none!
da analyze - none!

However, the above result is incorrect (at least for x86), there should be a dependency from from the store to the load, because the memory location written by the store overlap with the memory location read by the load.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 4, 2021
@sebpop
Copy link
Contributor

sebpop commented Nov 15, 2024

The current implementation of the dependence analysis test does not handle access functions reading/writing a different number of bytes. There are ways to extend DA to check for all the covered bytes by each R/W, however that would require more code for little benefit. This is really a corner case that does not occur often in codes.

The fix is to disable such R/W memory accesses as "aliasing". I will post a patch like this:

--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -722,9 +722,14 @@ static AliasResult underlyingObjectsAlias(AAResults *AA, LoopInfo *LI,
   const Value *AObj = getUnderlyingObject(LocA.Ptr);
   const Value *BObj = getUnderlyingObject(LocB.Ptr);

-  // If the underlying objects are the same, they must alias.
-  if (AObj == BObj)
+  if (AObj == BObj) {
+    // The dependence test gets confused if the size of the memory accesses differ.
+    if (LocA.Size != LocB.Size)
+      return AliasResult::MayAlias;
+
+    // If the underlying objects are the same, they must alias.
     return AliasResult::MustAlias;
+  }

sebpop added a commit to sebpop/llvm-project that referenced this issue Nov 15, 2024
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
sebpop added a commit to sebpop/llvm-project that referenced this issue Nov 18, 2024
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
sebpop added a commit to sebpop/llvm-project that referenced this issue Nov 25, 2024
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
sebpop added a commit to sebpop/llvm-project that referenced this issue Jan 18, 2025
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
sebpop added a commit to sebpop/llvm-project that referenced this issue Jan 30, 2025
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
sebpop added a commit to sebpop/llvm-project that referenced this issue Feb 19, 2025
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
sebpop added a commit to sebpop/llvm-project that referenced this issue Feb 26, 2025
…sizes

This fixes bug llvm#16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
@sebpop
Copy link
Contributor

sebpop commented May 20, 2025

Fixed with #123436

@sebpop sebpop closed this as completed May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla llvm:analysis
Projects
None yet
Development

No branches or pull requests

2 participants