Skip to content

[LAA] Consider accessed addrspace when mapping underlying obj to access. #129087

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

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Feb 27, 2025

In some cases, it is possible for the same underlying object to be accessed via pointers to different address spaces. This could lead to pointers from different address spaces ending up in the same dependency set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include the accessing address space.

Fixes #124759.

In some cases, it is possible for the same underlying object to be
accessed via pointers to different address spaces. This could lead to
pointers from different address spaces ending up in the same dependency
set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include
the accessing address space.

Fixes llvm#124759.
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Florian Hahn (fhahn)

Changes

In some cases, it is possible for the same underlying object to be accessed via pointers to different address spaces. This could lead to pointers from different address spaces ending up in the same dependency set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include the accessing address space.

Fixes #124759.


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/LoopAccessAnalysis.cpp (+8-4)
  • (added) llvm/test/Analysis/LoopAccessAnalysis/underlying-object-different-address-spaces.ll (+39)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index cf3bb6a8eae1c..38ee82b77a946 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1362,8 +1362,10 @@ void AccessAnalysis::processMemAccesses() {
 
     bool SetHasWrite = false;
 
-    // Map of pointers to last access encountered.
-    typedef DenseMap<const Value*, MemAccessInfo> UnderlyingObjToAccessMap;
+    // Map of (pointer to underlying objects, accessed address space) to last
+    // access encountered.
+    typedef DenseMap<std::pair<const Value *, unsigned>, MemAccessInfo>
+        UnderlyingObjToAccessMap;
     UnderlyingObjToAccessMap ObjToLastAccess;
 
     // Set of access to check after all writes have been processed.
@@ -1443,8 +1445,10 @@ void AccessAnalysis::processMemAccesses() {
                     UnderlyingObj->getType()->getPointerAddressSpace()))
               continue;
 
-            auto [It, Inserted] =
-                ObjToLastAccess.try_emplace(UnderlyingObj, Access);
+            auto [It, Inserted] = ObjToLastAccess.try_emplace(
+                {UnderlyingObj,
+                 cast<PointerType>(Ptr->getType())->getAddressSpace()},
+                Access);
             if (!Inserted) {
               DepCands.unionSets(Access, It->second);
               It->second = Access;
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/underlying-object-different-address-spaces.ll b/llvm/test/Analysis/LoopAccessAnalysis/underlying-object-different-address-spaces.ll
new file mode 100644
index 0000000000000..adf73c091be00
--- /dev/null
+++ b/llvm/test/Analysis/LoopAccessAnalysis/underlying-object-different-address-spaces.ll
@@ -0,0 +1,39 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes='print<access-info>' -disable-output %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+; Test case for https://github.com/llvm/llvm-project/issues/124759. The same
+; underlying object is access through pointers with different address spaces.
+define void @same_underlying_object_different_address_spaces(ptr %dst1.as1, ptr %dst2.as1) {
+; CHECK-LABEL: 'same_underlying_object_different_address_spaces'
+; CHECK-NEXT:    loop:
+; CHECK-NEXT:      Report: cannot identify array bounds
+; CHECK-NEXT:      Dependences:
+; CHECK-NEXT:      Run-time memory checks:
+; CHECK-NEXT:      Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:      SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Expressions re-written:
+;
+entry:
+  %alloc = alloca i8, i64 0, align 128
+  %as3 = addrspacecast ptr %alloc to ptr addrspace(3)
+  %as4 = addrspacecast ptr %alloc to ptr addrspace(4)
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]
+  store i32 0, ptr addrspace(4) %as4, align 4
+  store i32 0, ptr %dst1.as1, align 4
+  %l = load i64, ptr addrspace(3) %as3, align 4
+  store i64 %l, ptr %dst2.as1, align 4
+  %iv.next = add i64 %iv, 1
+  %c = icmp eq i64 %iv.next, 100
+  br i1 %c, label %loop, label %exit
+
+exit:
+  ret void
+}

@coldav
Copy link

coldav commented Feb 27, 2025

Could we cherry-pick this to llvm 20 please?

Copy link
Member

@Meinersbur Meinersbur left a comment

Choose a reason for hiding this comment

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

LGTM

@fhahn fhahn merged commit 275baed into llvm:main Feb 28, 2025
13 checks passed
@fhahn fhahn deleted the laa-check-address-space-for-dep-set branch February 28, 2025 20:56
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 28, 2025
…obj to access. (#129087)

In some cases, it is possible for the same underlying object to be
accessed via pointers to different address spaces. This could lead to
pointers from different address spaces ending up in the same dependency
set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include
the accessing address space.

Fixes llvm/llvm-project#124759.

PR: llvm/llvm-project#129087
fhahn added a commit to fhahn/llvm-project that referenced this pull request Feb 28, 2025
…ss. (llvm#129087)

In some cases, it is possible for the same underlying object to be
accessed via pointers to different address spaces. This could lead to
pointers from different address spaces ending up in the same dependency
set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include
the accessing address space.

Fixes llvm#124759.

PR: llvm#129087
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Mar 18, 2025
…ss. (llvm#129087)

In some cases, it is possible for the same underlying object to be
accessed via pointers to different address spaces. This could lead to
pointers from different address spaces ending up in the same dependency
set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include
the accessing address space.

Fixes llvm#124759.

PR: llvm#129087
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
…ss. (llvm#129087)

In some cases, it is possible for the same underlying object to be
accessed via pointers to different address spaces. This could lead to
pointers from different address spaces ending up in the same dependency
set, which isn't allowed (and triggers an assertion).

Update the mapping from underlying object -> last access to also include
the accessing address space.

Fixes llvm#124759.

PR: llvm#129087
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.

LoopVectorizer assert(AddressSpace == AS && with addrspacecast is hit
4 participants