[llvm] [SPIR-V] Fixup storage class for global private (PR #116636)
Nathan Gauër via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 18 07:51:37 PST 2024
https://github.com/Keenuts updated https://github.com/llvm/llvm-project/pull/116636
>From e5f24f9c20687b695cd2a5e80fc02f2095bd3fc8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nathan=20Gau=C3=ABr?= <brioche at google.com>
Date: Mon, 18 Nov 2024 15:46:53 +0100
Subject: [PATCH 1/2] [SPIR-V] Fixup storage class for global private
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
A global private variable was emitted with a Function storage class.
This should be Private.
This PR touches 2 other parts:
- a test
- SPIRVUtils.
The test for SPV_INTEL_function_pointers was testing the storage class
of a pointer, and expected "Function". But from what I read, the value
should be "CodeSectionINTEL". So since this test was already wrong,
updated it to expect "Private", which is as wrong as "Function".
Maybe this test should be removed or marked as X-FAIL?
SPIRVUtils: I needed to add AS->SC conversion for Private.
Input was already present, but not Output, so added Output just before
Private. Input/Output/Private were not used, so picked the next
numbers.
Signed-off-by: Nathan Gauër <brioche at google.com>
---
.../Target/SPIRV/SPIRVInstructionSelector.cpp | 23 +++++++++++--------
llvm/lib/Target/SPIRV/SPIRVUtils.cpp | 4 ++++
llvm/lib/Target/SPIRV/SPIRVUtils.h | 4 ++++
.../SPV_INTEL_function_pointers/fp_const.ll | 6 ++++-
.../pointers/variables-storage-class-vk.ll | 15 ++++++++++++
.../SPIRV/pointers/variables-storage-class.ll | 23 +++++++++++++++----
6 files changed, 59 insertions(+), 16 deletions(-)
create mode 100644 llvm/test/CodeGen/SPIRV/pointers/variables-storage-class-vk.ll
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 158314d23393ae..05a4bb12b54cba 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -3264,9 +3264,15 @@ bool SPIRVInstructionSelector::selectGlobalValue(
PointerBaseType = GR.getOrCreateSPIRVType(
GVType, MIRBuilder, SPIRV::AccessQualifier::ReadWrite, false);
}
- SPIRVType *ResType = GR.getOrCreateSPIRVPointerType(
- PointerBaseType, I, TII,
- addressSpaceToStorageClass(GV->getAddressSpace(), STI));
+
+ const unsigned AddrSpace = GV->getAddressSpace();
+ SPIRV::StorageClass::StorageClass StorageClass =
+ addressSpaceToStorageClass(AddrSpace, STI);
+ if (StorageClass == SPIRV::StorageClass::Function)
+ StorageClass = SPIRV::StorageClass::Private;
+
+ SPIRVType *ResType =
+ GR.getOrCreateSPIRVPointerType(PointerBaseType, I, TII, StorageClass);
std::string GlobalIdent;
if (!GV->hasName()) {
@@ -3337,11 +3343,8 @@ bool SPIRVInstructionSelector::selectGlobalValue(
if (HasInit && !Init)
return true;
- unsigned AddrSpace = GV->getAddressSpace();
- SPIRV::StorageClass::StorageClass Storage =
- addressSpaceToStorageClass(AddrSpace, STI);
bool HasLnkTy = GV->getLinkage() != GlobalValue::InternalLinkage &&
- Storage != SPIRV::StorageClass::Function;
+ StorageClass != SPIRV::StorageClass::Function;
SPIRV::LinkageType::LinkageType LnkType =
(GV->isDeclaration() || GV->hasAvailableExternallyLinkage())
? SPIRV::LinkageType::Import
@@ -3350,9 +3353,9 @@ bool SPIRVInstructionSelector::selectGlobalValue(
? SPIRV::LinkageType::LinkOnceODR
: SPIRV::LinkageType::Export);
- Register Reg = GR.buildGlobalVariable(ResVReg, ResType, GlobalIdent, GV,
- Storage, Init, GlobalVar->isConstant(),
- HasLnkTy, LnkType, MIRBuilder, true);
+ Register Reg = GR.buildGlobalVariable(
+ ResVReg, ResType, GlobalIdent, GV, StorageClass, Init,
+ GlobalVar->isConstant(), HasLnkTy, LnkType, MIRBuilder, true);
return Reg.isValid();
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index aeb2c29f7b8618..bfa3f50b8f16d9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -205,6 +205,10 @@ addressSpaceToStorageClass(unsigned AddrSpace, const SPIRVSubtarget &STI) {
: SPIRV::StorageClass::CrossWorkgroup;
case 7:
return SPIRV::StorageClass::Input;
+ case 8:
+ return SPIRV::StorageClass::Output;
+ case 9:
+ return SPIRV::StorageClass::Private;
default:
report_fatal_error("Unknown address space");
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.h b/llvm/lib/Target/SPIRV/SPIRVUtils.h
index 298b0b93b0e4d2..1a3c1399c5a02c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.h
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.h
@@ -166,6 +166,10 @@ storageClassToAddressSpace(SPIRV::StorageClass::StorageClass SC) {
return 6;
case SPIRV::StorageClass::Input:
return 7;
+ case SPIRV::StorageClass::Output:
+ return 8;
+ case SPIRV::StorageClass::Private:
+ return 9;
default:
report_fatal_error("Unable to get address space id");
}
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_const.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_const.ll
index b4faba9a4eb8e3..a6b5e2fe3c2971 100644
--- a/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_const.ll
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_const.ll
@@ -11,7 +11,11 @@
; CHECK-DAG: %[[TyFun:.*]] = OpTypeFunction %[[TyInt64]] %[[TyInt64]]
; CHECK-DAG: %[[TyInt8:.*]] = OpTypeInt 8 0
; CHECK-DAG: %[[TyPtrFun:.*]] = OpTypePointer Function %[[TyFun]]
-; CHECK-DAG: %[[ConstFunFp:.*]] = OpConstantFunctionPointerINTEL %[[TyPtrFun]] %[[DefFunFp:.*]]
+
+; FIXME: OpConstantFunctionPointerINTEL requires a CodeSectionINTEL ptr.
+; CHECK-DAG: %[[TyPtrPriv:.*]] = OpTypePointer Private %[[TyFun]]
+; CHECK-DAG: %[[ConstFunFp:.*]] = OpConstantFunctionPointerINTEL %[[TyPtrPriv]] %[[DefFunFp:.*]]
+
; CHECK-DAG: %[[TyPtrPtrFun:.*]] = OpTypePointer Function %[[TyPtrFun]]
; CHECK-DAG: %[[TyPtrInt8:.*]] = OpTypePointer Function %[[TyInt8]]
; CHECK-DAG: %[[TyPtrPtrInt8:.*]] = OpTypePointer Function %[[TyPtrInt8]]
diff --git a/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class-vk.ll b/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class-vk.ll
new file mode 100644
index 00000000000000..f7dbef2d01f5fc
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class-vk.ll
@@ -0,0 +1,15 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-vulkan1.3-compute %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-vulkan1.3-compute %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: %[[#U32:]] = OpTypeInt 32 0
+
+; CHECK-DAG: %[[#VAL:]] = OpConstant %[[#U32]] 456
+; CHECK-DAG: %[[#VTYPE:]] = OpTypePointer Private %[[#U32]]
+; CHECK-DAG: %[[#VAR:]] = OpVariable %[[#VTYPE]] Private %[[#VAL]]
+; CHECK-NOT: OpDecorate %[[#VAR]] LinkageAttributes
+ at PrivInternal = internal global i32 456
+
+define void @main() {
+ %l = load i32, ptr @PrivInternal
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class.ll b/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class.ll
index 2d4c805ac9df15..fdbe2750a0c159 100644
--- a/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class.ll
+++ b/llvm/test/CodeGen/SPIRV/pointers/variables-storage-class.ll
@@ -1,18 +1,31 @@
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+; CHECK-DAG: %[[#U8:]] = OpTypeInt 8 0
+; CHECK-DAG: %[[#U32:]] = OpTypeInt 32 0
+
+; CHECK-DAG: %[[#TYPE:]] = OpTypePointer CrossWorkgroup %[[#U8]]
+; CHECK-DAG: %[[#VAL:]] = OpConstantNull %[[#TYPE]]
+; CHECK-DAG: %[[#VTYPE:]] = OpTypePointer CrossWorkgroup %[[#TYPE]]
+; CHECK-DAG: %[[#PTR:]] = OpVariable %[[#VTYPE]] CrossWorkgroup %[[#VAL]]
@Ptr = addrspace(1) global ptr addrspace(1) null
- at Init = private addrspace(2) constant i32 123
-; CHECK-DAG: %[[#PTR:]] = OpVariable %[[#]] UniformConstant %[[#]]
-; CHECK-DAG: %[[#INIT:]] = OpVariable %[[#]] CrossWorkgroup %[[#]]
+; CHECK-DAG: %[[#VAL:]] = OpConstant %[[#U32]] 123
+; CHECK-DAG: %[[#VTYPE:]] = OpTypePointer UniformConstant %[[#U32]]
+; CHECK-DAG: %[[#INIT:]] = OpVariable %[[#VTYPE]] UniformConstant %[[#VAL]]
+ at Init = private addrspace(2) constant i32 123
-; CHECK: %[[#]] = OpLoad %[[#]] %[[#INIT]] Aligned 8
-; CHECK: OpCopyMemorySized %[[#]] %[[#PTR]] %[[#]] Aligned 4
+; CHECK-DAG: %[[#VAL:]] = OpConstant %[[#U32]] 456
+; CHECK-DAG: %[[#VTYPE:]] = OpTypePointer Private %[[#U32]]
+; CHECK-DAG: %[[#]] = OpVariable %[[#VTYPE]] Private %[[#VAL]]
+ at PrivInternal = internal global i32 456
define spir_kernel void @Foo() {
+; CHECK: %[[#]] = OpLoad %[[#]] %[[#PTR]] Aligned 8
%l = load ptr addrspace(1), ptr addrspace(1) @Ptr, align 8
+; CHECK: OpCopyMemorySized %[[#]] %[[#INIT]] %[[#]] Aligned 4
call void @llvm.memcpy.p1.p2.i64(ptr addrspace(1) align 4 %l, ptr addrspace(2) align 1 @Init, i64 4, i1 false)
+
ret void
}
>From f831d50eb28932e55104736663168f5112f88779 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nathan=20Gau=C3=ABr?= <brioche at google.com>
Date: Mon, 18 Nov 2024 16:36:15 +0100
Subject: [PATCH 2/2] pr feedback
---
llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 05a4bb12b54cba..a8643432cc27fa 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -3343,8 +3343,7 @@ bool SPIRVInstructionSelector::selectGlobalValue(
if (HasInit && !Init)
return true;
- bool HasLnkTy = GV->getLinkage() != GlobalValue::InternalLinkage &&
- StorageClass != SPIRV::StorageClass::Function;
+ bool HasLnkTy = GV->getLinkage() != GlobalValue::InternalLinkage;
SPIRV::LinkageType::LinkageType LnkType =
(GV->isDeclaration() || GV->hasAvailableExternallyLinkage())
? SPIRV::LinkageType::Import
More information about the llvm-commits
mailing list