Skip to content

Commit 2e8228d

Browse files
spencer-lunargspencer-lunarg
committed
Fix OpStore of BDA to align to 8
1 parent 0036567 commit 2e8228d

5 files changed

Lines changed: 102 additions & 8 deletions

File tree

‎SPIRV/SpvPostProcess.cpp‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -353,16 +353,26 @@ void Builder::postProcess(Instruction& inst)
353353
// Merge new and old (mis)alignment
354354
alignment |= inst.getImmediateOperand(alignmentIdx);
355355

356-
if (!is_store) {
357-
Instruction* inst_type = module.getInstruction(inst.getTypeId());
356+
Instruction* inst_type = nullptr;
357+
if (is_store) {
358+
// For a store, we are storing through a pointer, so what this ends up looking like when storing a BDA pointer is we have a "OpTypePointer PhysicalStorageBuffer" pointing at the actual "OpTypePointer PhysicalStorageBuffer" value storing
359+
// https://godbolt.org/z/58ff4P3sG
360+
inst_type = module.getInstruction(accessChain->getTypeId());
358361
if (inst_type->getOpCode() == Op::OpTypePointer &&
359362
inst_type->getImmediateOperand(0) == StorageClass::PhysicalStorageBuffer) {
360-
// This means we are loading a pointer which means need to ensure it is at least 8-byte aligned
361-
// See https://github.com/KhronosGroup/glslang/issues/4084
362-
// In case the alignment is currently 4, need to ensure it is 8 before grabbing the LSB
363-
alignment |= 8;
364-
alignment &= 8;
363+
inst_type = module.getInstruction(inst_type->getIdOperand(1));
365364
}
365+
} else {
366+
inst_type = module.getInstruction(inst.getTypeId());
367+
}
368+
369+
if (inst_type->getOpCode() == Op::OpTypePointer &&
370+
inst_type->getImmediateOperand(0) == StorageClass::PhysicalStorageBuffer) {
371+
// This means we are loading a pointer which means need to ensure it is at least 8-byte aligned
372+
// See https://github.com/KhronosGroup/glslang/issues/4084
373+
// In case the alignment is currently 4, need to ensure it is 8 before grabbing the LSB
374+
alignment |= 8;
375+
alignment &= 8;
366376
}
367377

368378
// Pick the LSB

‎Test/baseResults/spv.bufferhandle22.frag.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ spv.bufferhandle22.frag
470470
279: 278(ptr) AccessChain 273 274
471471
280: 6(ptr) CompositeExtract 277 0
472472
282: 281(ptr) AccessChain 279 35
473-
Store 282 280 Aligned 16
473+
Store 282 280 Aligned 8
474474
283: 6(ptr) Load 32(payload)
475475
286: 285(ptr) AccessChain 52(InBuffer) 284
476476
287: 49(S7) Load 286

‎Test/baseResults/spv.bufferhandle26.frag.out‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
spv.bufferhandle26.frag
2+
// Module Version 10000
3+
// Generated by (magic number): 8000b
4+
// Id's are bound by 27
5+
6+
Capability Shader
7+
Capability PhysicalStorageBufferAddressesEXT
8+
Extension "SPV_KHR_physical_storage_buffer"
9+
1: ExtInstImport "GLSL.std.450"
10+
MemoryModel PhysicalStorageBuffer64EXT GLSL450
11+
EntryPoint Fragment 4 "main"
12+
ExecutionMode 4 OriginUpperLeft
13+
Source GLSL 460
14+
SourceExtension "GL_EXT_buffer_reference"
15+
SourceExtension "GL_EXT_scalar_block_layout"
16+
Name 4 "main"
17+
Name 7 "constants"
18+
MemberName 7(constants) 0 "u_param"
19+
Name 9 "ParametersBuffer"
20+
MemberName 9(ParametersBuffer) 0 "src"
21+
MemberName 9(ParametersBuffer) 1 "dst"
22+
Name 12 "DataBuffer"
23+
MemberName 12(DataBuffer) 0 "data"
24+
Name 14 ""
25+
Decorate 7(constants) Block
26+
MemberDecorate 7(constants) 0 Offset 0
27+
Decorate 9(ParametersBuffer) Block
28+
MemberDecorate 9(ParametersBuffer) 0 Offset 0
29+
MemberDecorate 9(ParametersBuffer) 1 Offset 8
30+
Decorate 12(DataBuffer) Block
31+
MemberDecorate 12(DataBuffer) 0 NonWritable
32+
MemberDecorate 12(DataBuffer) 0 Offset 0
33+
2: TypeVoid
34+
3: TypeFunction 2
35+
TypeForwardPointer 6 PhysicalStorageBufferEXT
36+
7(constants): TypeStruct 6
37+
TypeForwardPointer 8 PhysicalStorageBufferEXT
38+
9(ParametersBuffer): TypeStruct 8 8
39+
10: TypeFloat 32
40+
11: TypeVector 10(float) 4
41+
12(DataBuffer): TypeStruct 11(fvec4)
42+
8: TypePointer PhysicalStorageBufferEXT 12(DataBuffer)
43+
6: TypePointer PhysicalStorageBufferEXT 9(ParametersBuffer)
44+
13: TypePointer PushConstant 7(constants)
45+
14: 13(ptr) Variable PushConstant
46+
15: TypeInt 32 1
47+
16: 15(int) Constant 0
48+
17: TypePointer PushConstant 6(ptr)
49+
20: 15(int) Constant 1
50+
23: TypePointer PhysicalStorageBufferEXT 8(ptr)
51+
4(main): 2 Function None 3
52+
5: Label
53+
18: 17(ptr) AccessChain 14 16
54+
19: 6(ptr) Load 18
55+
21: 17(ptr) AccessChain 14 16
56+
22: 6(ptr) Load 21
57+
24: 23(ptr) AccessChain 22 16
58+
25: 8(ptr) Load 24 Aligned 8
59+
26: 23(ptr) AccessChain 19 20
60+
Store 26 25 Aligned 8
61+
Return
62+
FunctionEnd

‎Test/spv.bufferhandle26.frag‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#version 460 core
2+
3+
#extension GL_EXT_buffer_reference: require
4+
#extension GL_EXT_scalar_block_layout: require
5+
6+
layout (buffer_reference, buffer_reference_align = 4, scalar) readonly buffer DataBuffer {
7+
vec4 data;
8+
};
9+
10+
layout (buffer_reference, buffer_reference_align = 8, scalar) buffer ParametersBuffer {
11+
DataBuffer src;
12+
DataBuffer dst;
13+
};
14+
15+
layout(push_constant) uniform constants {
16+
ParametersBuffer u_param;
17+
};
18+
19+
void main() {
20+
u_param.dst = u_param.src;
21+
}

‎gtests/Spv.FromFile.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ INSTANTIATE_TEST_SUITE_P(
393393
"spv.bufferhandle23.frag",
394394
"spv.bufferhandle24.frag",
395395
"spv.bufferhandle25.frag",
396+
"spv.bufferhandle26.frag",
396397
"spv.bufferhandleRuntimeArray.frag",
397398
"spv.bufferhandleUvec2.frag",
398399
"spv.bufferhandle_Error.frag",

0 commit comments

Comments
 (0)