Skip to content

Commit 5a07774

Browse files
[SPIR-V] Improve how lowering of formal arguments in SPIR-V Backend interprets a value of 'kernel_arg_type' (#78730)
The goal of this PR is to tolerate differences between description of formal arguments by function metadata (represented by "kernel_arg_type") and LLVM actual parameter types. A compiler may use "kernel_arg_type" of function metadata fields to encode detailed type information, whereas LLVM IR may utilize for an actual parameter a more general type, in particular, opaque pointer type. This PR proposes to resolve this by a fallback to LLVM actual parameter types during the lowering of formal function arguments in cases when the type can't be created by string content of "kernel_arg_type", i.e., when "kernel_arg_type" contains a type unknown for the SPIR-V Backend. An example of the issue manifestation is https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/main/test/transcoding/KernelArgTypeInOpString.ll, where a compiler generates for the following kernel function detailed `kernel_arg_type` info in a form of `!{!"image_kernel_data*", !"myInt", !"struct struct_name*"}`, and in LLVM IR same arguments are referred to as `@foo(ptr addrspace(1) %in, i32 %out, ptr addrspace(1) %outData)`. Both definitions are correct, and the resulting LLVM IR is correct, but lowering stage of SPIR-V Backend fails to generate SPIR-V type. ``` typedef int myInt; typedef struct { int width; int height; } image_kernel_data; struct struct_name { int i; int y; }; void kernel foo(__global image_kernel_data* in, __global struct struct_name *outData, myInt out) {} ``` ``` define spir_kernel void @foo(ptr addrspace(1) %in, i32 %out, ptr addrspace(1) %outData) ... !kernel_arg_type !7 ... { entry: ret void } ... !7 = !{!"image_kernel_data*", !"myInt", !"struct struct_name*"} ``` The PR changes a contract of `SPIRVType *getArgSPIRVType(...)` in a way that it may return `nullptr` to signal that the metadata string content is not recognized, so corresponding comments are added and a couple of checks for `nullptr` are inserted where appropriate.
1 parent e624648 commit 5a07774

File tree

5 files changed

+64
-20
lines changed

5 files changed

+64
-20
lines changed

llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1386,6 +1386,11 @@ static bool generateSampleImageInst(const StringRef DemangledCall,
13861386
ReturnType = ReturnType.substr(0, ReturnType.find('('));
13871387
}
13881388
SPIRVType *Type = GR->getOrCreateSPIRVTypeByName(ReturnType, MIRBuilder);
1389+
if (!Type) {
1390+
std::string DiagMsg =
1391+
"Unable to recognize SPIRV type name: " + ReturnType;
1392+
report_fatal_error(DiagMsg.c_str());
1393+
}
13891394
MRI->setRegClass(Call->Arguments[0], &SPIRV::IDRegClass);
13901395
MRI->setRegClass(Call->Arguments[1], &SPIRV::IDRegClass);
13911396
MRI->setRegClass(Call->Arguments[3], &SPIRV::IDRegClass);

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,22 +157,22 @@ static SPIRVType *getArgSPIRVType(const Function &F, unsigned ArgIdx,
157157
isSpecialOpaqueType(OriginalArgType))
158158
return GR->getOrCreateSPIRVType(OriginalArgType, MIRBuilder, ArgAccessQual);
159159

160-
MDString *MDKernelArgType = getOCLKernelArgType(F, ArgIdx);
161-
if (!MDKernelArgType || (!MDKernelArgType->getString().ends_with("*") &&
162-
!MDKernelArgType->getString().ends_with("_t")))
163-
return GR->getOrCreateSPIRVType(OriginalArgType, MIRBuilder, ArgAccessQual);
164-
165-
if (MDKernelArgType->getString().ends_with("*"))
166-
return GR->getOrCreateSPIRVTypeByName(
167-
MDKernelArgType->getString(), MIRBuilder,
168-
addressSpaceToStorageClass(OriginalArgType->getPointerAddressSpace()));
169-
170-
if (MDKernelArgType->getString().ends_with("_t"))
171-
return GR->getOrCreateSPIRVTypeByName(
172-
"opencl." + MDKernelArgType->getString().str(), MIRBuilder,
173-
SPIRV::StorageClass::Function, ArgAccessQual);
174-
175-
llvm_unreachable("Unable to recognize argument type name.");
160+
SPIRVType *ResArgType = nullptr;
161+
if (MDString *MDKernelArgType = getOCLKernelArgType(F, ArgIdx)) {
162+
StringRef MDTypeStr = MDKernelArgType->getString();
163+
if (MDTypeStr.ends_with("*"))
164+
ResArgType = GR->getOrCreateSPIRVTypeByName(
165+
MDTypeStr, MIRBuilder,
166+
addressSpaceToStorageClass(
167+
OriginalArgType->getPointerAddressSpace()));
168+
else if (MDTypeStr.ends_with("_t"))
169+
ResArgType = GR->getOrCreateSPIRVTypeByName(
170+
"opencl." + MDTypeStr.str(), MIRBuilder,
171+
SPIRV::StorageClass::Function, ArgAccessQual);
172+
}
173+
return ResArgType ? ResArgType
174+
: GR->getOrCreateSPIRVType(OriginalArgType, MIRBuilder,
175+
ArgAccessQual);
176176
}
177177

178178
static bool isEntryPoint(const Function &F) {

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,8 +443,9 @@ Register SPIRVGlobalRegistry::buildConstantSampler(
443443
SPIRVType *SampTy;
444444
if (SpvType)
445445
SampTy = getOrCreateSPIRVType(getTypeForSPIRVType(SpvType), MIRBuilder);
446-
else
447-
SampTy = getOrCreateSPIRVTypeByName("opencl.sampler_t", MIRBuilder);
446+
else if ((SampTy = getOrCreateSPIRVTypeByName("opencl.sampler_t",
447+
MIRBuilder)) == nullptr)
448+
report_fatal_error("Unable to recognize SPIRV type name: opencl.sampler_t");
448449

449450
auto Sampler =
450451
ResReg.isValid()
@@ -942,6 +943,7 @@ SPIRVGlobalRegistry::checkSpecialInstr(const SPIRV::SpecialTypeDescriptor &TD,
942943
return nullptr;
943944
}
944945

946+
// Returns nullptr if unable to recognize SPIRV type name
945947
// TODO: maybe use tablegen to implement this.
946948
SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVTypeByName(
947949
StringRef TypeStr, MachineIRBuilder &MIRBuilder,
@@ -993,8 +995,10 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVTypeByName(
993995
} else if (TypeStr.starts_with("double")) {
994996
Ty = Type::getDoubleTy(Ctx);
995997
TypeStr = TypeStr.substr(strlen("double"));
996-
} else
997-
llvm_unreachable("Unable to recognize SPIRV type name.");
998+
} else {
999+
// Unable to recognize SPIRV type name
1000+
return nullptr;
1001+
}
9981002

9991003
auto SpirvTy = getOrCreateSPIRVType(Ty, MIRBuilder, AQ);
10001004

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ class SPIRVGlobalRegistry {
138138

139139
// Either generate a new OpTypeXXX instruction or return an existing one
140140
// corresponding to the given string containing the name of the builtin type.
141+
// Return nullptr if unable to recognize SPIRV type name from `TypeStr`.
141142
SPIRVType *getOrCreateSPIRVTypeByName(
142143
StringRef TypeStr, MachineIRBuilder &MIRBuilder,
143144
SPIRV::StorageClass::StorageClass SC = SPIRV::StorageClass::Function,
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
2+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
3+
4+
; CHECK: %[[TyInt:.*]] = OpTypeInt 8 0
5+
; CHECK: %[[TyPtr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyInt]]
6+
; CHECK: OpFunctionParameter %[[TyPtr]]
7+
; CHECK: OpFunctionParameter %[[TyPtr]]
8+
9+
%struct.my_kernel_data = type { i32, i32, i32, i32, i32 }
10+
%struct.my_struct = type { i32, i32 }
11+
12+
define spir_kernel void @test(ptr addrspace(1) %in, ptr addrspace(1) %outData) !kernel_arg_type !5 {
13+
entry:
14+
ret void
15+
}
16+
17+
!llvm.module.flags = !{!0}
18+
!opencl.enable.FP_CONTRACT = !{}
19+
!opencl.ocl.version = !{!1}
20+
!opencl.spir.version = !{!2}
21+
!opencl.used.extensions = !{!3}
22+
!opencl.used.optional.core.features = !{!3}
23+
!opencl.compiler.options = !{!3}
24+
!llvm.ident = !{!4}
25+
!opencl.kernels = !{!6}
26+
27+
!0 = !{i32 1, !"wchar_size", i32 4}
28+
!1 = !{i32 1, i32 0}
29+
!2 = !{i32 1, i32 2}
30+
!3 = !{}
31+
!4 = !{!"clang version 6.0.0"}
32+
!5 = !{!"my_kernel_data*", !"struct my_struct*"}
33+
!6 = !{ptr @test}
34+

0 commit comments

Comments
 (0)