[llvm] [SPIR-V] Fixup storage class for global private (PR #116636)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 18 07:08:06 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-spir-v
Author: Nathan Gauër (Keenuts)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/116636.diff
6 Files Affected:
- (modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+13-10)
- (modified) llvm/lib/Target/SPIRV/SPIRVUtils.cpp (+4)
- (modified) llvm/lib/Target/SPIRV/SPIRVUtils.h (+4)
- (modified) llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_const.ll (+5-1)
- (added) llvm/test/CodeGen/SPIRV/pointers/variables-storage-class-vk.ll (+15)
- (modified) llvm/test/CodeGen/SPIRV/pointers/variables-storage-class.ll (+18-5)
``````````diff
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
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/116636
More information about the llvm-commits
mailing list