Skip to content
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

Jump to bottom

[Lint][AMDGPU] No store to const addrspace #109181

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+56 −2
Open

[Lint][AMDGPU] No store to const addrspace #109181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a099028
[Verifier][AMDGPU] No store to const addrspace
jofrn Sep 18, 2024
4553484
[Lint] Switch check to Linter
jofrn Sep 20, 2024
393311c
Add negative test, move isConstantAddressSpace
jofrn Sep 20, 2024
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
27 changes: 27 additions & 0 deletions llvm/include/llvm/Support/AMDGPUAddrSpace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,33 @@ inline bool isExtendedGlobalAddrSpace(unsigned AS) {
AS == AMDGPUAS::CONSTANT_ADDRESS_32BIT ||
AS > AMDGPUAS::MAX_AMDGPU_ADDRESS;
}

inline bool isConstantAddressSpace(unsigned AS) {
switch(AS) {
using namespace AMDGPUAS;
case CONSTANT_ADDRESS:
case CONSTANT_ADDRESS_32BIT:
case CONSTANT_BUFFER_0:
case CONSTANT_BUFFER_1:
case CONSTANT_BUFFER_2:
case CONSTANT_BUFFER_3:
case CONSTANT_BUFFER_4:
case CONSTANT_BUFFER_5:
case CONSTANT_BUFFER_6:
case CONSTANT_BUFFER_7:
case CONSTANT_BUFFER_8:
case CONSTANT_BUFFER_9:
case CONSTANT_BUFFER_10:
case CONSTANT_BUFFER_11:
case CONSTANT_BUFFER_12:
case CONSTANT_BUFFER_13:
case CONSTANT_BUFFER_14:
case CONSTANT_BUFFER_15:
return true;
default:
return false;
}
}
} // end namespace AMDGPU

} // end namespace llvm
Expand Down
10 changes: 8 additions & 2 deletions llvm/lib/Analysis/Lint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
#include "llvm/IR/PassManager.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/Value.h"
#include "llvm/Support/AMDGPUAddrSpace.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/KnownBits.h"
#include "llvm/Support/raw_ostream.h"
Expand Down Expand Up @@ -121,9 +122,9 @@ class Lint : public InstVisitor<Lint> {
Value *findValue(Value *V, bool OffsetOk) const;
Value *findValueImpl(Value *V, bool OffsetOk,
SmallPtrSetImpl<Value *> &Visited) const;

public:
Module *Mod;
Triple TT;
const DataLayout *DL;
AliasAnalysis *AA;
AssumptionCache *AC;
Expand All @@ -135,7 +136,8 @@ class Lint : public InstVisitor<Lint> {

Lint(Module *Mod, const DataLayout *DL, AliasAnalysis *AA,
AssumptionCache *AC, DominatorTree *DT, TargetLibraryInfo *TLI)
: Mod(Mod), DL(DL), AA(AA), AC(AC), DT(DT), TLI(TLI),
: Mod(Mod), TT(Triple::normalize(Mod->getTargetTriple())),
DL(DL), AA(AA), AC(AC), DT(DT), TLI(TLI),
MessagesStr(Messages) {}

void WriteValues(ArrayRef<const Value *> Vs) {
Expand Down Expand Up @@ -401,6 +403,10 @@ void Lint::visitMemoryReference(Instruction &I, const MemoryLocation &Loc,
"Unusual: Address one pointer dereference", &I);

if (Flags & MemRef::Write) {
if (TT.isAMDGPU())
Check(!AMDGPU::isConstantAddressSpace(UnderlyingObject->getType()->getPointerAddressSpace()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible or better to do the check via TTI? I'm not sure if it's a good idea to do the check based on triple.

"Undefined behavior: Write to const memory", &I);

if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(UnderlyingObject))
Check(!GV->isConstant(), "Undefined behavior: Write to read-only memory",
&I);
Expand Down
21 changes: 21 additions & 0 deletions llvm/test/Analysis/Lint/const-store.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
; RUN: not opt --mtriple=amdgcn --passes=lint --lint-abort-on-error %s -o /dev/null |& FileCheck %s
; RUN: opt --mtriple=amdgcn --mcpu=gfx1030 --passes=lint %s -o /dev/null |& FileCheck %s --check-prefixes=CHECK,CHECK0
; RUN: opt --mtriple=x86_64 --passes=lint --lint-abort-on-error %s -o /dev/null |& FileCheck %s --allow-empty --check-prefix=NOERR
; NOERR: {{^$}}

Copy link
Contributor

@shiltian shiltian Sep 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think you will need ; REQUIRES: amdgpu-registered-target as well as other targets that you want to test.

define amdgpu_kernel void @store_const(ptr addrspace(4) %out, i32 %a, i32 %b) {
; CHECK: Undefined behavior: Write to const memory
; CHECK-NEXT: store i32 %r, ptr addrspace(4) %out
%r = add i32 %a, %b
store i32 %r, ptr addrspace(4) %out
ret void
}

define amdgpu_kernel void @memcpy_to_const(ptr addrspace(6) %dst, ptr %src) {
; CHECK0: Undefined behavior: Write to const memory
; CHECK0-NEXT: call void @llvm.memcpy.p6.p0.i32(ptr addrspace(6) %dst, ptr %src, i32 256, i1 false)
call void @llvm.memcpy.px.py.i32(ptr addrspace(6) %dst, ptr %src, i32 256, i1 false)
ret void
}

declare void @llvm.memcpy.px.py.i32(ptr addrspace(6) noalias nocapture writeonly, ptr noalias nocapture readonly, i32, i1 immarg)
Loading