[llvm] 5251176 - [AMDGPU] Fix SSID merge in SIMemoryLegalizer to use smallest inclusiv… (#208300)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 13 06:23:31 PDT 2026
Author: Wooseok Lee
Date: 2026-07-13T08:23:26-05:00
New Revision: 52511761bc80e96b7d07b4da71bb08c8f498dee1
URL: https://github.com/llvm/llvm-project/commit/52511761bc80e96b7d07b4da71bb08c8f498dee1
DIFF: https://github.com/llvm/llvm-project/commit/52511761bc80e96b7d07b4da71bb08c8f498dee1.diff
LOG: [AMDGPU] Fix SSID merge in SIMemoryLegalizer to use smallest inclusiv… (#208300)
…e scope
SIMemOpAccess::constructFromMIWithMMO merged multiple MMO sync scopes by
calling isSyncScopeInclusion(A, B) and blindly overwriting SSID with B
when it returned false. This was correct when B strictly subsumes A, but
isSyncScopeInclusion returned false for both "A < B" and "A and B are
incomparable" -- e.g. agent-one-as (one-AS, agent level) vs workgroup
(cross-AS, workgroup level) live on different branches of the
two-dimensional scope lattice and neither subsumes the other. In the
incomparable case the resulting SSID depended on MMO order, silently
dropping agent-scope cache-management requirements.
Fix: replace isSyncScopeInclusion (std::optional<bool>) with
getMergedSyncScopeID (std::optional<SyncScope::ID>) which returns the
smallest scope inclusive of both inputs. The merged level is
max(level_A, level_B); if either input is cross-AS the result is also
cross-AS, since dropping cross-AS ordering would silently lose the
ordering guarantee for address spaces not covered by a one-AS scope.
Add tests in memory-legalizer-multiple-mem-operands-atomics.mir
verifying that a FLAT_ATOMIC_ADD with agent-one-as and workgroup MMOs
produces agent-scope cache ops (BUFFER_WBINVL1_VOL) regardless of MMO
order.
Added:
Modified:
llvm/lib/Target/AMDGPU/AMDGPUMachineModuleInfo.h
llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
llvm/test/CodeGen/AMDGPU/av-invalid-scope.ll
llvm/test/CodeGen/AMDGPU/memory-legalizer-invalid-syncscope.ll
llvm/test/CodeGen/AMDGPU/memory-legalizer-multiple-mem-operands-atomics.mir
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUMachineModuleInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUMachineModuleInfo.h
index bf852bb38376e..6aee1bba71335 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUMachineModuleInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUMachineModuleInfo.h
@@ -47,47 +47,6 @@ class AMDGPUMachineModuleInfo final : public MachineModuleInfoELF {
/// Cluster synchronization scope ID (single address space).
SyncScope::ID ClusterOneAddressSpaceSSID;
- /// In AMDGPU target synchronization scopes are inclusive, meaning a
- /// larger synchronization scope is inclusive of a smaller synchronization
- /// scope.
- ///
- /// \returns \p SSID's inclusion ordering, or "std::nullopt" if \p SSID is not
- /// supported by the AMDGPU target.
- std::optional<uint8_t>
- getSyncScopeInclusionOrdering(SyncScope::ID SSID) const {
- if (SSID == SyncScope::SingleThread ||
- SSID == getSingleThreadOneAddressSpaceSSID())
- return 0;
- else if (SSID == getWavefrontSSID() ||
- SSID == getWavefrontOneAddressSpaceSSID())
- return 1;
- else if (SSID == getWorkgroupSSID() ||
- SSID == getWorkgroupOneAddressSpaceSSID())
- return 2;
- else if (SSID == getClusterSSID() ||
- SSID == getClusterOneAddressSpaceSSID())
- return 3;
- else if (SSID == getAgentSSID() ||
- SSID == getAgentOneAddressSpaceSSID())
- return 4;
- else if (SSID == SyncScope::System ||
- SSID == getSystemOneAddressSpaceSSID())
- return 5;
-
- return std::nullopt;
- }
-
- /// \returns True if \p SSID is restricted to single address space, false
- /// otherwise
- bool isOneAddressSpace(SyncScope::ID SSID) const {
- return SSID == getClusterOneAddressSpaceSSID() ||
- SSID == getSingleThreadOneAddressSpaceSSID() ||
- SSID == getWavefrontOneAddressSpaceSSID() ||
- SSID == getWorkgroupOneAddressSpaceSSID() ||
- SSID == getAgentOneAddressSpaceSSID() ||
- SSID == getSystemOneAddressSpaceSSID();
- }
-
public:
AMDGPUMachineModuleInfo(const MachineModuleInfo &MMI);
@@ -130,26 +89,57 @@ class AMDGPUMachineModuleInfo final : public MachineModuleInfoELF {
return ClusterOneAddressSpaceSSID;
}
- /// In AMDGPU target synchronization scopes are inclusive, meaning a
- /// larger synchronization scope is inclusive of a smaller synchronization
- /// scope.
+ /// In AMDGPU, synchronization scopes are inclusive: a larger scope is
+ /// inclusive of a smaller one (e.g. agent includes workgroup).
+ ///
+ /// Returns the merged synchronization scope of \p A and \p B: the smallest
+ /// scope that is inclusive of both. Takes the larger inclusion level and,
+ /// if either scope is cross-address-space, the result is also
+ /// cross-address-space (since a one-AS scope cannot subsume a cross-AS
+ /// scope at the same level).
///
- /// \returns True if synchronization scope \p A is larger than or equal to
- /// synchronization scope \p B, false if synchronization scope \p A is smaller
- /// than synchronization scope \p B, or "std::nullopt" if either
- /// synchronization scope \p A or \p B is not supported by the AMDGPU target.
- std::optional<bool> isSyncScopeInclusion(SyncScope::ID A,
- SyncScope::ID B) const {
- const auto &AIO = getSyncScopeInclusionOrdering(A);
- const auto &BIO = getSyncScopeInclusionOrdering(B);
- if (!AIO || !BIO)
+ /// \returns The merged scope ID, or "std::nullopt" if either scope is not
+ /// supported by the AMDGPU target.
+ std::optional<SyncScope::ID> getMergedSyncScopeID(SyncScope::ID A,
+ SyncScope::ID B) const {
+ // Ordered from smallest to largest scope. Level is the index.
+ // Cross-AS and one-AS scopes share the same inclusion ordering level.
+ // Level | Cross-AS scope | One-AS scope
+ // ------+------------------+----------------------
+ // 0 | singlethread | singlethread-one-as
+ // 1 | wavefront | wavefront-one-as
+ // 2 | workgroup | workgroup-one-as
+ // 3 | cluster | cluster-one-as
+ // 4 | agent | agent-one-as
+ // 5 | system | one-as
+ const SyncScope::ID CrossAS[] = {
+ SyncScope::SingleThread, getWavefrontSSID(), getWorkgroupSSID(),
+ getClusterSSID(), getAgentSSID(), SyncScope::System};
+ const SyncScope::ID OneAS[] = {
+ getSingleThreadOneAddressSpaceSSID(), getWavefrontOneAddressSpaceSSID(),
+ getWorkgroupOneAddressSpaceSSID(), getClusterOneAddressSpaceSSID(),
+ getAgentOneAddressSpaceSSID(), getSystemOneAddressSpaceSSID()};
+
+ // Returns {level, isOneAS} for a given scope, or nullopt if unsupported.
+ auto GetLevelAndOneAS =
+ [&](SyncScope::ID SSID) -> std::optional<std::pair<unsigned, bool>> {
+ for (auto [I, Cross, One] : llvm::enumerate(CrossAS, OneAS)) {
+ if (Cross == SSID)
+ return std::make_pair(I, false);
+ if (One == SSID)
+ return std::make_pair(I, true);
+ }
return std::nullopt;
+ };
- bool IsAOneAddressSpace = isOneAddressSpace(A);
- bool IsBOneAddressSpace = isOneAddressSpace(B);
+ auto AI = GetLevelAndOneAS(A);
+ auto BI = GetLevelAndOneAS(B);
+ if (!AI || !BI)
+ return std::nullopt;
- return *AIO >= *BIO &&
- (IsAOneAddressSpace == IsBOneAddressSpace || !IsAOneAddressSpace);
+ unsigned Level = std::max(AI->first, BI->first);
+ // If either scope is cross-AS, the result must be cross-AS.
+ return (AI->second && BI->second) ? OneAS[Level] : CrossAS[Level];
}
};
diff --git a/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp b/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
index 432946b092425..12f73d8caf630 100644
--- a/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
+++ b/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
@@ -833,7 +833,7 @@ std::optional<SIMemOpInfo> SIMemOpAccess::constructFromMIWithMMO(
const MachineBasicBlock::iterator &MI) const {
assert(MI->getNumMemOperands() > 0);
- SyncScope::ID SSID = SyncScope::SingleThread;
+ std::optional<SyncScope::ID> MergedSSID;
AtomicOrdering Ordering = AtomicOrdering::NotAtomic;
AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic;
SIAtomicAddrSpace InstrAddrSpace = SIAtomicAddrSpace::NONE;
@@ -849,19 +849,19 @@ std::optional<SIMemOpInfo> SIMemOpAccess::constructFromMIWithMMO(
IsVolatile |= MMO->isVolatile();
IsLastUse |= MMO->getFlags() & MOLastUse;
IsCooperative |= MMO->getFlags() & MOCooperative;
- InstrAddrSpace |=
- toSIAtomicAddrSpace(MMO->getPointerInfo().getAddrSpace());
+ InstrAddrSpace |= toSIAtomicAddrSpace(MMO->getPointerInfo().getAddrSpace());
AtomicOrdering OpOrdering = MMO->getSuccessOrdering();
if (OpOrdering != AtomicOrdering::NotAtomic) {
- const auto &IsSyncScopeInclusion =
- MMI->isSyncScopeInclusion(SSID, MMO->getSyncScopeID());
- if (!IsSyncScopeInclusion) {
- reportUnsupported(MI,
- "Unsupported non-inclusive atomic synchronization scope");
+ // Merge the accumulated scope with the new one to get the smallest scope
+ // inclusive of both.
+ SyncScope::ID CurSSID = MergedSSID.value_or(MMO->getSyncScopeID());
+ const auto &Merged =
+ MMI->getMergedSyncScopeID(CurSSID, MMO->getSyncScopeID());
+ if (!Merged) {
+ reportUnsupported(MI, "Unsupported atomic synchronization scope");
return std::nullopt;
}
-
- SSID = *IsSyncScopeInclusion ? SSID : MMO->getSyncScopeID();
+ MergedSSID = *Merged;
Ordering = getMergedAtomicOrdering(Ordering, OpOrdering);
assert(MMO->getFailureOrdering() != AtomicOrdering::Release &&
MMO->getFailureOrdering() != AtomicOrdering::AcquireRelease);
@@ -869,6 +869,7 @@ std::optional<SIMemOpInfo> SIMemOpAccess::constructFromMIWithMMO(
getMergedAtomicOrdering(FailureOrdering, MMO->getFailureOrdering());
}
}
+ SyncScope::ID SSID = MergedSSID.value_or(SyncScope::SingleThread);
// FIXME: The MMO of buffer atomic instructions does not always have an atomic
// ordering. We only need to handle VBUFFER atomics on GFX12+ so we can fix it
diff --git a/llvm/test/CodeGen/AMDGPU/av-invalid-scope.ll b/llvm/test/CodeGen/AMDGPU/av-invalid-scope.ll
index faaae26142232..a2d9a3b4faecf 100644
--- a/llvm/test/CodeGen/AMDGPU/av-invalid-scope.ll
+++ b/llvm/test/CodeGen/AMDGPU/av-invalid-scope.ll
@@ -1,8 +1,8 @@
; RUN: not llc -global-isel=0 -mtriple=amdgpu9.42 -filetype=null < %s 2>&1 | FileCheck %s
; RUN: not llc -global-isel=1 -mtriple=amdgpu9.42 -filetype=null < %s 2>&1 | FileCheck %s
-; CHECK: error: {{.*}}: in function av_load_bad_scope {{.*}}: Unsupported non-inclusive atomic synchronization scope
-; CHECK: error: {{.*}}: in function av_store_bad_scope {{.*}}: Unsupported non-inclusive atomic synchronization scope
+; CHECK: error: {{.*}}: in function av_load_bad_scope {{.*}}: Unsupported atomic synchronization scope
+; CHECK: error: {{.*}}: in function av_store_bad_scope {{.*}}: Unsupported atomic synchronization scope
define <4 x i32> @av_load_bad_scope(ptr addrspace(1) %p) {
%v = call <4 x i32> @llvm.amdgcn.av.load.b128.p1(ptr addrspace(1) %p, metadata !0)
diff --git a/llvm/test/CodeGen/AMDGPU/memory-legalizer-invalid-syncscope.ll b/llvm/test/CodeGen/AMDGPU/memory-legalizer-invalid-syncscope.ll
index 1a2058cbe39e4..e820bac9e5ae5 100644
--- a/llvm/test/CodeGen/AMDGPU/memory-legalizer-invalid-syncscope.ll
+++ b/llvm/test/CodeGen/AMDGPU/memory-legalizer-invalid-syncscope.ll
@@ -11,7 +11,7 @@ entry:
ret void
}
-; CHECK: error: <unknown>:0:0: in function invalid_load void (ptr, ptr): Unsupported non-inclusive atomic synchronization scope
+; CHECK: error: <unknown>:0:0: in function invalid_load void (ptr, ptr): Unsupported atomic synchronization scope
define amdgpu_kernel void @invalid_load(
ptr %in, ptr %out) {
entry:
@@ -20,7 +20,7 @@ entry:
ret void
}
-; CHECK: error: <unknown>:0:0: in function invalid_store void (i32, ptr): Unsupported non-inclusive atomic synchronization scope
+; CHECK: error: <unknown>:0:0: in function invalid_store void (i32, ptr): Unsupported atomic synchronization scope
define amdgpu_kernel void @invalid_store(
i32 %in, ptr %out) {
entry:
@@ -28,7 +28,7 @@ entry:
ret void
}
-; CHECK: error: <unknown>:0:0: in function invalid_cmpxchg void (ptr, i32, i32): Unsupported non-inclusive atomic synchronization scope
+; CHECK: error: <unknown>:0:0: in function invalid_cmpxchg void (ptr, i32, i32): Unsupported atomic synchronization scope
define amdgpu_kernel void @invalid_cmpxchg(
ptr %out, i32 %in, i32 %old) {
entry:
@@ -37,7 +37,7 @@ entry:
ret void
}
-; CHECK: error: <unknown>:0:0: in function invalid_rmw void (ptr, i32): Unsupported non-inclusive atomic synchronization scope
+; CHECK: error: <unknown>:0:0: in function invalid_rmw void (ptr, i32): Unsupported atomic synchronization scope
define amdgpu_kernel void @invalid_rmw(
ptr %out, i32 %in) {
entry:
diff --git a/llvm/test/CodeGen/AMDGPU/memory-legalizer-multiple-mem-operands-atomics.mir b/llvm/test/CodeGen/AMDGPU/memory-legalizer-multiple-mem-operands-atomics.mir
index 3941da208f825..1496f2533d88b 100644
--- a/llvm/test/CodeGen/AMDGPU/memory-legalizer-multiple-mem-operands-atomics.mir
+++ b/llvm/test/CodeGen/AMDGPU/memory-legalizer-multiple-mem-operands-atomics.mir
@@ -111,3 +111,49 @@ body: |
S_ENDPGM 0
...
+---
+# Test that multi-MMO atomics with incomparable sync scopes (agent-one-as and
+# workgroup) produce agent-scope cache ops regardless of MMO order. The merged
+# scope of agent-one-as and workgroup is agent (cross-AS). Before the fix, the
+# result was MMO-order dependent.
+name: merge_agent_one_as_and_workgroup
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3
+ ; GCN-LABEL: name: merge_agent_one_as_and_workgroup
+ ; GCN: liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3
+ ; GCN-NEXT: {{ $}}
+ ; GCN-NEXT: renamable $vgpr0_vgpr1 = COPY $sgpr0_sgpr1
+ ; GCN-NEXT: renamable $vgpr2 = COPY $sgpr2
+ ; GCN-NEXT: S_WAITCNT_soft .Vmcnt_0_Lgkmcnt_0
+ ; GCN-NEXT: FLAT_ATOMIC_ADD $vgpr0_vgpr1, $vgpr2, 0, 0, implicit $exec, implicit $flat_scr :: (load store syncscope("agent-one-as") seq_cst (s32) on `ptr addrspace(1) poison`, addrspace 1), (load store syncscope("workgroup") seq_cst (s32) on `ptr addrspace(1) poison`, addrspace 1)
+ ; GCN-NEXT: S_WAITCNT_soft .Vmcnt_0
+ ; GCN-NEXT: BUFFER_WBINVL1_VOL implicit $exec
+ ; GCN-NEXT: S_ENDPGM 0
+ renamable $vgpr0_vgpr1 = COPY $sgpr0_sgpr1
+ renamable $vgpr2 = COPY $sgpr2
+ FLAT_ATOMIC_ADD $vgpr0_vgpr1, $vgpr2, 0, 0, implicit $exec, implicit $flat_scr :: (load store syncscope("agent-one-as") seq_cst (s32) on `ptr addrspace(1) poison`), (load store syncscope("workgroup") seq_cst (s32) on `ptr addrspace(1) poison`)
+ S_ENDPGM 0
+...
+---
+name: merge_agent_one_as_and_workgroup_reversed
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3
+ ; GCN-LABEL: name: merge_agent_one_as_and_workgroup_reversed
+ ; GCN: liveins: $sgpr0, $sgpr1, $sgpr2, $sgpr3
+ ; GCN-NEXT: {{ $}}
+ ; GCN-NEXT: renamable $vgpr0_vgpr1 = COPY $sgpr0_sgpr1
+ ; GCN-NEXT: renamable $vgpr2 = COPY $sgpr2
+ ; GCN-NEXT: S_WAITCNT_soft .Vmcnt_0_Lgkmcnt_0
+ ; GCN-NEXT: FLAT_ATOMIC_ADD $vgpr0_vgpr1, $vgpr2, 0, 0, implicit $exec, implicit $flat_scr :: (load store syncscope("workgroup") seq_cst (s32) on `ptr addrspace(1) poison`, addrspace 1), (load store syncscope("agent-one-as") seq_cst (s32) on `ptr addrspace(1) poison`, addrspace 1)
+ ; GCN-NEXT: S_WAITCNT_soft .Vmcnt_0
+ ; GCN-NEXT: BUFFER_WBINVL1_VOL implicit $exec
+ ; GCN-NEXT: S_ENDPGM 0
+ renamable $vgpr0_vgpr1 = COPY $sgpr0_sgpr1
+ renamable $vgpr2 = COPY $sgpr2
+ FLAT_ATOMIC_ADD $vgpr0_vgpr1, $vgpr2, 0, 0, implicit $exec, implicit $flat_scr :: (load store syncscope("workgroup") seq_cst (s32) on `ptr addrspace(1) poison`), (load store syncscope("agent-one-as") seq_cst (s32) on `ptr addrspace(1) poison`)
+ S_ENDPGM 0
+...
More information about the llvm-commits
mailing list