Skip to content

Commit 8732d0f

Browse files
authored
[SPIRV] Don't add CPacked and Alignement decorations for Vulkan (#138711)
Packed structs get a different layout than a struct that is not packed. This is handled by assigning different offset decoration when appropriate. The `CPacked` decoration is not required, and is in fact not valid when creating a shader. Similaraly the alignment decoration is not allows when creating a shader. We must avoid generating that decoration. Fixes #138268
1 parent 7c57e82 commit 8732d0f

File tree

4 files changed

+126
-7
lines changed

4 files changed

+126
-7
lines changed

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ bool SPIRVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
319319
buildOpDecorate(VRegs[i][0], MIRBuilder,
320320
SPIRV::Decoration::MaxByteOffset, {DerefBytes});
321321
}
322-
if (Arg.hasAttribute(Attribute::Alignment)) {
322+
if (Arg.hasAttribute(Attribute::Alignment) && !ST->isVulkanEnv()) {
323323
auto Alignment = static_cast<unsigned>(
324324
Arg.getAttribute(Attribute::Alignment).getValueAsInt());
325325
buildOpDecorate(VRegs[i][0], MIRBuilder, SPIRV::Decoration::Alignment,

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ Register SPIRVGlobalRegistry::buildGlobalVariable(
770770
if (IsConst && ST.isOpenCLEnv())
771771
buildOpDecorate(Reg, MIRBuilder, SPIRV::Decoration::Constant, {});
772772

773-
if (GVar && GVar->getAlign().valueOrOne().value() != 1) {
773+
if (GVar && GVar->getAlign().valueOrOne().value() != 1 && !ST.isVulkanEnv()) {
774774
unsigned Alignment = (unsigned)GVar->getAlign().valueOrOne().value();
775775
buildOpDecorate(Reg, MIRBuilder, SPIRV::Decoration::Alignment, {Alignment});
776776
}
@@ -799,6 +799,9 @@ static std::string GetSpirvImageTypeName(const SPIRVType *Type,
799799
const std::string &Prefix,
800800
SPIRVGlobalRegistry &GR);
801801

802+
// Returns a name based on the Type. Notes that this does not look at
803+
// decorations, and will return the same string for two types that are the same
804+
// except for decorations.
802805
static std::string buildSpirvTypeName(const SPIRVType *Type,
803806
MachineIRBuilder &MIRBuilder,
804807
SPIRVGlobalRegistry &GR) {
@@ -885,9 +888,9 @@ Register SPIRVGlobalRegistry::getOrCreateGlobalVariableWithBinding(
885888
Register VarReg =
886889
MIRBuilder.getMRI()->createVirtualRegister(&SPIRV::iIDRegClass);
887890

888-
// TODO: The name should come from the llvm-ir, but how that name will be
889-
// passed from the HLSL to the backend has not been decided. Using this place
890-
// holder for now.
891+
// TODO(138533): The name should come from the llvm-ir, but how that name will
892+
// be passed from the HLSL to the backend has not been decided. Using this
893+
// place holder for now.
891894
std::string Name =
892895
("__resource_" + buildSpirvTypeName(VarType, MIRBuilder, *this) + "_" +
893896
Twine(Set) + "_" + Twine(Binding))
@@ -955,6 +958,8 @@ SPIRVType *SPIRVGlobalRegistry::getOpTypeStruct(
955958
const StructType *Ty, MachineIRBuilder &MIRBuilder,
956959
SPIRV::AccessQualifier::AccessQualifier AccQual,
957960
bool ExplicitLayoutRequired, bool EmitIR) {
961+
const SPIRVSubtarget &ST =
962+
cast<SPIRVSubtarget>(MIRBuilder.getMF().getSubtarget());
958963
SmallVector<Register, 4> FieldTypes;
959964
constexpr unsigned MaxWordCount = UINT16_MAX;
960965
const size_t NumElements = Ty->getNumElements();
@@ -977,7 +982,7 @@ SPIRVType *SPIRVGlobalRegistry::getOpTypeStruct(
977982
Register ResVReg = createTypeVReg(MIRBuilder);
978983
if (Ty->hasName())
979984
buildOpName(ResVReg, Ty->getName(), MIRBuilder);
980-
if (Ty->isPacked())
985+
if (Ty->isPacked() && !ST.isVulkanEnv())
981986
buildOpDecorate(ResVReg, MIRBuilder, SPIRV::Decoration::CPacked, {});
982987

983988
SPIRVType *SPVType =
@@ -1629,7 +1634,8 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVTypeByName(
16291634
// Unable to recognize SPIRV type name
16301635
return nullptr;
16311636

1632-
auto SpirvTy = getOrCreateSPIRVType(Ty, MIRBuilder, AQ, false, true);
1637+
const SPIRVType *SpirvTy =
1638+
getOrCreateSPIRVType(Ty, MIRBuilder, AQ, false, true);
16331639

16341640
// Handle "type*" or "type* vector[N]".
16351641
if (TypeStr.consume_front("*"))
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
; Check names and decoration of global variables.
2+
3+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s -check-prefixes=CHECK,OCL
4+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
5+
6+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s -check-prefixes=CHECK,OCL
7+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
8+
9+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s -check-prefixes=CHECK,VK
10+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %}
11+
12+
; CHECK-DAG: OpName %[[#id18:]] "G1"
13+
; CHECK-DAG: OpName %[[#id22:]] "g1"
14+
; CHECK-DAG: OpName %[[#id23:]] "g2"
15+
; CHECK-DAG: OpName %[[#id27:]] "g4"
16+
; CHECK-DAG: OpName %[[#id30:]] "c1"
17+
; CHECK-DAG: OpName %[[#id31:]] "n_t"
18+
; CHECK-DAG: OpName %[[#id32:]] "w"
19+
; CHECK-DAG: OpName %[[#id34:]] "a.b"
20+
; CHECK-DAG: OpName %[[#id35:]] "e"
21+
; CHECK-DAG: OpName %[[#id36:]] "y.z"
22+
; CHECK-DAG: OpName %[[#id38:]] "x"
23+
24+
; CHECK-NOT: OpDecorate %[[#id18]] LinkageAttributes
25+
; OCL-DAG: OpDecorate %[[#id18]] Constant
26+
; OCL-DAG: OpDecorate %[[#id22]] Alignment 4
27+
; VK-NOT: OpDecorate {{.*}} Constant
28+
; VK-NOT: OpDecorate {{.*}} Alignment
29+
; CHECK-DAG: OpDecorate %[[#id22]] LinkageAttributes "g1" Export
30+
; OCL-DAG: OpDecorate %[[#id23]] Alignment 4
31+
; OCL-DAG: OpDecorate %[[#id27]] Alignment 4
32+
; VK-NOT: OpDecorate {{.*}} Constant
33+
; VK-NOT: OpDecorate {{.*}} Alignment
34+
; CHECK-DAG: OpDecorate %[[#id27]] LinkageAttributes "g4" Export
35+
; OCL-DAG: OpDecorate %[[#id30]] Constant
36+
; OCL-DAG: OpDecorate %[[#id30]] Alignment 4
37+
; VK-NOT: OpDecorate {{.*}} Constant
38+
; VK-NOT: OpDecorate {{.*}} Alignment
39+
; CHECK-DAG: OpDecorate %[[#id30]] LinkageAttributes "c1" Export
40+
; OCL-DAG: OpDecorate %[[#id31]] Constant
41+
; VK-NOT: OpDecorate {{.*}} Constant
42+
; VK-NOT: OpDecorate {{.*}} Alignment
43+
; CHECK-DAG: OpDecorate %[[#id31]] LinkageAttributes "n_t" Import
44+
; OCL-DAG: OpDecorate %[[#id32]] Constant
45+
; OCL-DAG: OpDecorate %[[#id32]] Alignment 4
46+
; VK-NOT: OpDecorate {{.*}} Constant
47+
; VK-NOT: OpDecorate {{.*}} Alignment
48+
; CHECK-DAG: OpDecorate %[[#id32]] LinkageAttributes "w" Export
49+
; OCL-DAG: OpDecorate %[[#id34]] Constant
50+
; OCL-DAG: OpDecorate %[[#id34]] Alignment 4
51+
; VK-NOT: OpDecorate {{.*}} Constant
52+
; VK-NOT: OpDecorate {{.*}} Alignment
53+
; CHECK-DAG: OpDecorate %[[#id35]] LinkageAttributes "e" Import
54+
; OCL-DAG: OpDecorate %[[#id36]] Alignment 4
55+
; OCL-DAG: OpDecorate %[[#id38]] Constant
56+
; OCL-DAG: OpDecorate %[[#id38]] Alignment 4
57+
; VK-NOT: OpDecorate {{.*}} Constant
58+
; VK-NOT: OpDecorate {{.*}} Alignment
59+
60+
%"class.sycl::_V1::nd_item" = type { i8 }
61+
62+
@G1 = private unnamed_addr addrspace(1) constant %"class.sycl::_V1::nd_item" poison, align 1
63+
@g1 = addrspace(1) global i32 1, align 4
64+
@g2 = internal addrspace(1) global i32 2, align 4
65+
@g4 = common addrspace(1) global i32 0, align 4
66+
@c1 = addrspace(2) constant [2 x i32] [i32 0, i32 1], align 4
67+
@n_t = external addrspace(2) constant [256 x i32]
68+
@w = addrspace(1) constant i32 0, align 4
69+
@a.b = internal addrspace(2) constant [2 x i32] [i32 2, i32 3], align 4
70+
@e = external addrspace(1) global i32
71+
@y.z = internal addrspace(1) global i32 0, align 4
72+
@x = internal addrspace(2) constant float 1.000000e+00, align 4
73+
74+
define internal spir_func void @foo() {
75+
ret void
76+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv1.6-vulkan1.3-library %s -o - | FileCheck %s
2+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv1.6-vulkan1.3-library %s -o - -filetype=obj | spirv-val %}
3+
4+
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1"
5+
6+
; CHECK-DAG: OpName [[unpacked:%[0-9]+]] "unpacked"
7+
; CHECK-DAG: OpName [[packed:%[0-9]+]] "packed"
8+
9+
; CHECK-NOT: OpDecorate {{.*}} CPacked
10+
; CHECK-DAG: OpMemberDecorate [[unpacked]] 0 Offset 0
11+
; CHECK-DAG: OpMemberDecorate [[unpacked]] 1 Offset 16
12+
13+
; CHECK-NOT: OpDecorate {{.*}} CPacked
14+
; CHECK-DAG: OpMemberDecorate [[packed]] 0 Offset 0
15+
; CHECK-DAG: OpMemberDecorate [[packed]] 1 Offset 4
16+
; CHECK-NOT: OpDecorate {{.*}} CPacked
17+
18+
19+
%unpacked = type {i32, <3 x i32>}
20+
%packed = type <{i32, <3 x i32>}>
21+
22+
23+
define external i32 @unpacked_vulkan_buffer_load() {
24+
entry:
25+
%handle = tail call target("spirv.VulkanBuffer", [0 x %unpacked], 12, 0) @llvm.spv.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, i1 false)
26+
%0 = tail call noundef nonnull align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer(target("spirv.VulkanBuffer", [0 x %unpacked], 12, 0) %handle, i32 1)
27+
%1 = load i32, ptr addrspace(11) %0, align 4
28+
ret i32 %1
29+
}
30+
31+
define external i32 @packed_vulkan_buffer_load() {
32+
entry:
33+
%handle = tail call target("spirv.VulkanBuffer", [0 x %packed], 12, 0) @llvm.spv.resource.handlefrombinding(i32 0, i32 1, i32 1, i32 0, i1 false)
34+
%0 = tail call noundef nonnull align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer(target("spirv.VulkanBuffer", [0 x %packed], 12, 0) %handle, i32 1)
35+
%1 = load i32, ptr addrspace(11) %0, align 4
36+
ret i32 %1
37+
}

0 commit comments

Comments
 (0)