[llvm] 53d33d3 - [Attributor] Fix an issue that an access is skipped by mistake (#101862)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 18:29:44 PDT 2024
Author: Shilei Tian
Date: 2024-08-06T21:29:39-04:00
New Revision: 53d33d3ba5eedac8fccb9d36576cd667800a4a38
URL: https://github.com/llvm/llvm-project/commit/53d33d3ba5eedac8fccb9d36576cd667800a4a38
DIFF: https://github.com/llvm/llvm-project/commit/53d33d3ba5eedac8fccb9d36576cd667800a4a38.diff
LOG: [Attributor] Fix an issue that an access is skipped by mistake (#101862)
When we check if an access can be skipped, there is a case that an
inter-procedural interference access exists after a dominant write.
Currently we
rely on `AAInterFnReachability` to tell if the access can be reachable.
If it is
not, we can safely skip the access. However, it is based on an
assumption that
the AA exists. It is possible that the AA doesn't exist. In this case,
we can't
safely assume the acess can be skipped because we have to assume the
access can
reach. This can happen when `AAInterFnReachability` is not in the
allowed AA
list when creating the attributor, such as AMDGPUAttributor.
Co-authored-by: Mark de Wever <koraq at xs4all.nl>
Added:
llvm/test/CodeGen/AMDGPU/indirect-call-set-from-other-function.ll
Modified:
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 77026c6aa5b272..db5e94806e9a16 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -1325,20 +1325,20 @@ struct AAPointerInfoImpl
const auto *FnReachabilityAA = A.getAAFor<AAInterFnReachability>(
QueryingAA, IRPosition::function(Scope), DepClassTy::OPTIONAL);
-
- // Without going backwards in the call tree, can we reach the access
- // from the least dominating write. Do not allow to pass the instruction
- // itself either.
- bool Inserted = ExclusionSet.insert(&I).second;
-
- if (!FnReachabilityAA ||
- !FnReachabilityAA->instructionCanReach(
- A, *LeastDominatingWriteInst,
- *Acc.getRemoteInst()->getFunction(), &ExclusionSet))
- WriteChecked = true;
-
- if (Inserted)
- ExclusionSet.erase(&I);
+ if (FnReachabilityAA) {
+ // Without going backwards in the call tree, can we reach the access
+ // from the least dominating write. Do not allow to pass the
+ // instruction itself either.
+ bool Inserted = ExclusionSet.insert(&I).second;
+
+ if (!FnReachabilityAA->instructionCanReach(
+ A, *LeastDominatingWriteInst,
+ *Acc.getRemoteInst()->getFunction(), &ExclusionSet))
+ WriteChecked = true;
+
+ if (Inserted)
+ ExclusionSet.erase(&I);
+ }
}
if (ReadChecked && WriteChecked)
diff --git a/llvm/test/CodeGen/AMDGPU/indirect-call-set-from-other-function.ll b/llvm/test/CodeGen/AMDGPU/indirect-call-set-from-other-function.ll
new file mode 100644
index 00000000000000..f419d89a7f0a44
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/indirect-call-set-from-other-function.ll
@@ -0,0 +1,73 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-globals
+; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -passes=amdgpu-attributor %s -o - | FileCheck %s
+
+ at g_fn = addrspace(1) global ptr null
+
+;.
+; CHECK: @g_fn = addrspace(1) global ptr null
+;.
+define void @set_fn(ptr %fn) {
+; CHECK-LABEL: define {{[^@]+}}@set_fn
+; CHECK-SAME: (ptr [[FN:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: store ptr [[FN]], ptr addrspace(1) @g_fn, align 8
+; CHECK-NEXT: ret void
+;
+entry:
+ store ptr %fn, ptr addrspace(1) @g_fn
+ ret void
+}
+
+define void @get_fn(ptr %fn) {
+; CHECK-LABEL: define {{[^@]+}}@get_fn
+; CHECK-SAME: (ptr [[FN:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr addrspace(1) @g_fn, align 8
+; CHECK-NEXT: store ptr [[LOAD]], ptr [[FN]], align 8
+; CHECK-NEXT: ret void
+;
+entry:
+ %load = load ptr, ptr addrspace(1) @g_fn
+ store ptr %load, ptr %fn
+ ret void
+}
+
+define void @foo() {
+; CHECK-LABEL: define {{[^@]+}}@foo
+; CHECK-SAME: () #[[ATTR1:[0-9]+]] {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[FN:%.*]] = alloca ptr, align 8, addrspace(5)
+; CHECK-NEXT: store ptr null, ptr addrspace(5) [[FN]], align 8
+; CHECK-NEXT: [[FN_CAST:%.*]] = addrspacecast ptr addrspace(5) [[FN]] to ptr
+; CHECK-NEXT: call void @get_fn(ptr [[FN_CAST]])
+; CHECK-NEXT: [[LOAD:%.*]] = load ptr, ptr addrspace(5) [[FN]], align 8
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne ptr [[LOAD]], null
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: [[LOAD_1:%.*]] = load ptr, ptr addrspace(5) [[FN]], align 8
+; CHECK-NEXT: call void [[LOAD_1]]()
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: ret void
+;
+entry:
+ %fn = alloca ptr, addrspace(5)
+ store ptr null, ptr addrspace(5) %fn
+ %fn.cast = addrspacecast ptr addrspace(5) %fn to ptr
+ call void @get_fn(ptr %fn.cast)
+ %load = load ptr, ptr addrspace(5) %fn
+ %tobool = icmp ne ptr %load, null
+ br i1 %tobool, label %if.then, label %if.end
+
+if.then:
+ %load.1 = load ptr, ptr addrspace(5) %fn
+ call void %load.1()
+ br label %if.end
+
+if.end:
+ ret void
+}
+;.
+; CHECK: attributes #[[ATTR0]] = { "amdgpu-no-agpr" "amdgpu-no-completion-action" "amdgpu-no-default-queue" "amdgpu-no-dispatch-id" "amdgpu-no-dispatch-ptr" "amdgpu-no-heap-ptr" "amdgpu-no-hostcall-ptr" "amdgpu-no-implicitarg-ptr" "amdgpu-no-lds-kernel-id" "amdgpu-no-multigrid-sync-arg" "amdgpu-no-queue-ptr" "amdgpu-no-workgroup-id-x" "amdgpu-no-workgroup-id-y" "amdgpu-no-workgroup-id-z" "amdgpu-no-workitem-id-x" "amdgpu-no-workitem-id-y" "amdgpu-no-workitem-id-z" "amdgpu-waves-per-eu"="4,10" "uniform-work-group-size"="false" }
+; CHECK: attributes #[[ATTR1]] = { "amdgpu-waves-per-eu"="4,10" "uniform-work-group-size"="false" }
+;.
More information about the llvm-commits
mailing list