[llvm] [Verifier][AMDGPU] No store to const addrspace (PR #109181)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 20 11:17:11 PDT 2024
https://github.com/jofrn updated https://github.com/llvm/llvm-project/pull/109181
>From a0990280f4cfa70cd465b06847cb7e9b8a79fe8d Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Wed, 18 Sep 2024 12:34:36 -0700
Subject: [PATCH 1/2] [Verifier][AMDGPU] No store to const addrspace
Ensure store to const addrspace is not allowed by IR Verifier.
---
llvm/lib/IR/Verifier.cpp | 31 +++++++++++++++++++++++++
llvm/test/CodeGen/AMDGPU/const-store.ll | 9 +++++++
2 files changed, 40 insertions(+)
create mode 100644 llvm/test/CodeGen/AMDGPU/const-store.ll
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 06a67346fbf959..ca213a77baf42e 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -4224,6 +4224,33 @@ void Verifier::visitLoadInst(LoadInst &LI) {
visitInstruction(LI);
}
+static 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;
+ }
+}
+
void Verifier::visitStoreInst(StoreInst &SI) {
PointerType *PTy = dyn_cast<PointerType>(SI.getOperand(1)->getType());
Check(PTy, "Store operand must be a pointer.", &SI);
@@ -4246,6 +4273,10 @@ void Verifier::visitStoreInst(StoreInst &SI) {
Check(SI.getSyncScopeID() == SyncScope::System,
"Non-atomic store cannot have SynchronizationScope specified", &SI);
}
+ if (TT.isAMDGPU()) {
+ Check(!isConstantAddressSpace(SI.getPointerAddressSpace()),
+ "Store cannot be to constant addrspace", &SI);
+ }
visitInstruction(SI);
}
diff --git a/llvm/test/CodeGen/AMDGPU/const-store.ll b/llvm/test/CodeGen/AMDGPU/const-store.ll
new file mode 100644
index 00000000000000..9447ef9db59986
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/const-store.ll
@@ -0,0 +1,9 @@
+; RUN: not llc -mtriple=amdgcn < %s |& FileCheck %s
+
+define amdgpu_kernel void @store_const(ptr addrspace(4) %out, i32 %a, i32 %b) {
+; CHECK: Store cannot be to constant addrspace
+; CHECK-NEXT: store i32 %r, ptr addrspace(4) %out
+ %r = add i32 %a, %b
+ store i32 %r, ptr addrspace(4) %out
+ ret void
+}
>From b3fea0b07585e2da95c0033fe96820509ab16399 Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Fri, 20 Sep 2024 11:16:37 -0700
Subject: [PATCH 2/2] [Lint] Switch check to Linter
---
llvm/lib/Analysis/Lint.cpp | 39 ++++++++++++++++++++++++-
llvm/lib/IR/Verifier.cpp | 31 --------------------
llvm/test/CodeGen/AMDGPU/const-store.ll | 6 ++--
3 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/llvm/lib/Analysis/Lint.cpp b/llvm/lib/Analysis/Lint.cpp
index e0a029802bbd9a..ead7ad495db837 100644
--- a/llvm/lib/Analysis/Lint.cpp
+++ b/llvm/lib/Analysis/Lint.cpp
@@ -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"
@@ -122,8 +123,10 @@ class Lint : public InstVisitor<Lint> {
Value *findValueImpl(Value *V, bool OffsetOk,
SmallPtrSetImpl<Value *> &Visited) const;
+ bool isConstantAddressSpace(unsigned AS) const;
public:
Module *Mod;
+ Triple TT;
const DataLayout *DL;
AliasAnalysis *AA;
AssumptionCache *AC;
@@ -135,7 +138,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) {
@@ -378,6 +382,36 @@ void Lint::visitReturnInst(ReturnInst &I) {
}
}
+bool Lint::isConstantAddressSpace(unsigned AS) const {
+ if (TT.isAMDGPU()) {
+ 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;
+ }
+ }
+ return false;
+}
+
// TODO: Check that the reference is in bounds.
// TODO: Check readnone/readonly function attributes.
void Lint::visitMemoryReference(Instruction &I, const MemoryLocation &Loc,
@@ -401,6 +435,9 @@ void Lint::visitMemoryReference(Instruction &I, const MemoryLocation &Loc,
"Unusual: Address one pointer dereference", &I);
if (Flags & MemRef::Write) {
+ Check(!isConstantAddressSpace(Ptr->getType()->getPointerAddressSpace()),
+ "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);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index ca213a77baf42e..06a67346fbf959 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -4224,33 +4224,6 @@ void Verifier::visitLoadInst(LoadInst &LI) {
visitInstruction(LI);
}
-static 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;
- }
-}
-
void Verifier::visitStoreInst(StoreInst &SI) {
PointerType *PTy = dyn_cast<PointerType>(SI.getOperand(1)->getType());
Check(PTy, "Store operand must be a pointer.", &SI);
@@ -4273,10 +4246,6 @@ void Verifier::visitStoreInst(StoreInst &SI) {
Check(SI.getSyncScopeID() == SyncScope::System,
"Non-atomic store cannot have SynchronizationScope specified", &SI);
}
- if (TT.isAMDGPU()) {
- Check(!isConstantAddressSpace(SI.getPointerAddressSpace()),
- "Store cannot be to constant addrspace", &SI);
- }
visitInstruction(SI);
}
diff --git a/llvm/test/CodeGen/AMDGPU/const-store.ll b/llvm/test/CodeGen/AMDGPU/const-store.ll
index 9447ef9db59986..18e2fb5be86631 100644
--- a/llvm/test/CodeGen/AMDGPU/const-store.ll
+++ b/llvm/test/CodeGen/AMDGPU/const-store.ll
@@ -1,7 +1,7 @@
-; RUN: not llc -mtriple=amdgcn < %s |& FileCheck %s
+; RUN: not opt -mtriple=amdgcn --passes=lint --lint-abort-on-error %s -o - |& FileCheck %s
-define amdgpu_kernel void @store_const(ptr addrspace(4) %out, i32 %a, i32 %b) {
-; CHECK: Store cannot be to constant addrspace
+define amdgpu_kernel void @store_const(ptr addrspace(4) %out, i32 %a, i32 %b) addrspace(4) {
+; 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
More information about the llvm-commits
mailing list