Skip to content

[AMDGPU] Check noalias.addrspace in mayAccessScratchThroughFlat #151319

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+29 −23
Open

[AMDGPU] Check noalias.addrspace in mayAccessScratchThroughFlat #151319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56c9223
[AMDGPU] Check noalias.addrspace in mayAccessScratchThroughFlat
Pierre-vh Jul 30, 2025
fd716f3
Use shared helper
Pierre-vh Aug 1, 2025
757c736
comments
Pierre-vh Aug 4, 2025
269abdd
Comments
Pierre-vh Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 3 additions & 17 deletions llvm/lib/Target/AMDGPU/SIISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17713,23 +17713,9 @@ atomicSupportedIfLegalIntType(const AtomicRMWInst *RMW) {

/// Return if a flat address space atomicrmw can access private memory.
static bool flatInstrMayAccessPrivate(const Instruction *I) {
const MDNode *NoaliasAddrSpaceMD =
I->getMetadata(LLVMContext::MD_noalias_addrspace);
if (!NoaliasAddrSpaceMD)
return true;

for (unsigned I = 0, E = NoaliasAddrSpaceMD->getNumOperands() / 2; I != E;
++I) {
auto *Low = mdconst::extract<ConstantInt>(
NoaliasAddrSpaceMD->getOperand(2 * I + 0));
if (Low->getValue().uge(AMDGPUAS::PRIVATE_ADDRESS)) {
auto *High = mdconst::extract<ConstantInt>(
NoaliasAddrSpaceMD->getOperand(2 * I + 1));
return High->getValue().ule(AMDGPUAS::PRIVATE_ADDRESS);
}
}

return true;
const MDNode *MD = I->getMetadata(LLVMContext::MD_noalias_addrspace);
return !MD ||
!AMDGPU::hasValueInRangeLikeMetadata(*MD, AMDGPUAS::PRIVATE_ADDRESS);
}

TargetLowering::AtomicExpansionKind
Expand Down
9 changes: 6 additions & 3 deletions llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4266,12 +4266,15 @@ bool SIInstrInfo::mayAccessScratchThroughFlat(const MachineInstr &MI) const {
if (MI.memoperands_empty())
return true;

// TODO (?): Does this need to be taught how to read noalias.addrspace ?

// See if any memory operand specifies an address space that involves scratch.
return any_of(MI.memoperands(), [](const MachineMemOperand *Memop) {
unsigned AS = Memop->getAddrSpace();
return AS == AMDGPUAS::PRIVATE_ADDRESS || AS == AMDGPUAS::FLAT_ADDRESS;
if (AS == AMDGPUAS::FLAT_ADDRESS) {
const MDNode *MD = Memop->getAAInfo().NoAliasAddrSpace;
return !MD || !AMDGPU::hasValueInRangeLikeMetadata(
*MD, AMDGPUAS::PRIVATE_ADDRESS);
}
return AS == AMDGPUAS::PRIVATE_ADDRESS;
});
}

Expand Down
14 changes: 14 additions & 0 deletions llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "llvm/IR/IntrinsicsAMDGPU.h"
#include "llvm/IR/IntrinsicsR600.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Metadata.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/MC/MCRegisterInfo.h"
#include "llvm/MC/MCSubtargetInfo.h"
Expand Down Expand Up @@ -1666,6 +1667,19 @@ getIntegerVecAttribute(const Function &F, StringRef Name, unsigned Size) {
return Vals;
}

bool hasValueInRangeLikeMetadata(const MDNode &MD, int64_t Val) {
assert((MD.getNumOperands() % 2 == 0) && "invalid number of operands!");
for (unsigned I = 0, E = MD.getNumOperands() / 2; I != E; ++I) {
auto *Low = mdconst::extract<ConstantInt>(MD.getOperand(2 * I + 0));
auto *High = mdconst::extract<ConstantInt>(MD.getOperand(2 * I + 1));
assert(Low->getValue().ult(High->getValue()) && "invalid range metadata!");
Copy link
Contributor

Choose a reason for hiding this comment

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

This should already be checked by the IR verifier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can't check it's actually dealing with a range metadata, and not something else. The asserts are to avoid a silent failure in case this is passed some other type of MD accidentally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arsenm There is a test that has a "wrapped" range: llvm/test/Transforms/AtomicExpand/AMDGPU/expand-cmpxchg-flat-maybe-private.ll

noalias.addrspace there has a value of [6;5).
I guess this is a different way to say "everything is noalias except private" ?
If so then I need to replace that assert with a if/else to handle such ranges

if (Low->getValue().ule(Val) && High->getValue().ugt(Val))
return true;
}

return false;
}

unsigned getVmcntBitMask(const IsaVersion &Version) {
return (1 << (getVmcntBitWidthLo(Version.Major) +
getVmcntBitWidthHi(Version.Major))) -
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class MCInstrInfo;
class MCRegisterClass;
class MCRegisterInfo;
class MCSubtargetInfo;
class MDNode;
class StringRef;
class Triple;
class raw_ostream;
Expand Down Expand Up @@ -1064,6 +1065,9 @@ SmallVector<unsigned> getIntegerVecAttribute(const Function &F, StringRef Name,
std::optional<SmallVector<unsigned>>
getIntegerVecAttribute(const Function &F, StringRef Name, unsigned Size);

/// Checks if \p Val is inside \p MD, a !range-like metadata.
bool hasValueInRangeLikeMetadata(const MDNode &MD, int64_t Val);

/// Represents the counter values to wait for in an s_waitcnt instruction.
///
/// Large values (including the maximum possible integer) can be used to
Expand Down
5 changes: 2 additions & 3 deletions llvm/test/CodeGen/AMDGPU/gfx1250-scratch-scope-se.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,19 @@ define void @test_flat_store_no_scratch_alloc(ptr %ptr, i32 %val) #0 {
ret void
}

; TODO: handle
define void @test_flat_store_noalias_addrspace(ptr %ptr, i32 %val) {
; GCN-LABEL: test_flat_store_noalias_addrspace:
; GCN: ; %bb.0:
; GCN-NEXT: s_wait_loadcnt_dscnt 0x0
; GCN-NEXT: s_wait_kmcnt 0x0
; GCN-NEXT: flat_store_b32 v[0:1], v2 scope:SCOPE_SE
; GCN-NEXT: flat_store_b32 v[0:1], v2
; GCN-NEXT: s_wait_dscnt 0x0
; GCN-NEXT: s_set_pc_i64 s[30:31]
store i32 %val, ptr %ptr, !noalias.addrspace !{i32 5, i32 6}
ret void
}

; TODO: would be nice to handle too
; TODO: would be nice to handle
define void @test_flat_store_select(ptr addrspace(1) %a, ptr addrspace(3) %b, i1 %cond, i32 %val) {
; GCN-SDAG-LABEL: test_flat_store_select:
; GCN-SDAG: ; %bb.0:
Expand Down
Loading