Skip to content

Commit f0be268

Browse files
vainiovanoarcady-lunarg
authored andcommitted
Use typed access for descriptor heap-bound buffers
I believe that untyped pointer access to uniform or storage buffers requires using explicitly laid out types, but glslang cannot currently guarantee that accesses to buffers obtained through the descriptor heap would have such layout information. Instead, we can use typed pointers similarly to buffers provided through descriptor sets. Resolves: KhronosGroup#4224 Resolves: KhronosGroup#4225
1 parent 2d1b22e commit f0be268

5 files changed

Lines changed: 184 additions & 181 deletions

SPIRV/SpvBuilder.cpp

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3130,31 +3130,15 @@ Id Builder::createLoad(Id lValue, spv::Decoration precision, spv::MemoryAccessMa
31303130

31313131
Instruction* Builder::createDescHeapLoadStoreBaseRemap(Id baseId, Op op)
31323132
{
3133-
// could only be untypedAccessChain or BufferPointerEXT op.
3133+
// could only be untypedAccessChain op.
31343134
spv::Op instOp = module.getInstruction(baseId)->getOpCode();
31353135
spv::Id baseVal = baseId;
31363136
// base type (from run time array)
31373137
spv::Id resultTy = getIdOperand(baseId, 0);
31383138
// Descriptor heap using run time array.
31393139
if (accessChain.descHeapInfo.descHeapStorageClass != StorageClass::Max)
31403140
resultTy = getIdOperand(resultTy, 0);
3141-
if (instOp == Op::OpBufferPointerEXT) {
3142-
// get base structure type from run time array of buffer structure type.
3143-
// create an extra untyped access chain for buffer pointer.
3144-
resultTy = accessChain.descHeapInfo.descHeapBaseTy;
3145-
Instruction* chain = new Instruction(getUniqueId(), getTypeId(baseId), Op::OpUntypedAccessChainKHR);
3146-
// base type.
3147-
chain->addIdOperand(resultTy);
3148-
// base
3149-
chain->addIdOperand(baseId);
3150-
// index
3151-
for (int i = 0; i < (int)accessChain.indexChain.size(); ++i) {
3152-
chain->addIdOperand(accessChain.indexChain[i]);
3153-
}
3154-
addInstruction(std::unique_ptr<Instruction>(chain));
3155-
baseVal = chain->getResultId();
3156-
clearAccessChain();
3157-
} else if (instOp != Op::OpUntypedAccessChainKHR) {
3141+
if (instOp != Op::OpUntypedAccessChainKHR) {
31583142
assert(false && "Not a untyped load type");
31593143
}
31603144

@@ -3223,15 +3207,12 @@ Id Builder::createDescHeapAccessChain()
32233207
if (rsrcOffsetIdx != 0)
32243208
accessChain.base = accessChain.descHeapInfo.structRemappedBase;
32253209
Id base = accessChain.base;
3226-
Id untypedResultTy = accessChain.descHeapInfo.descHeapBaseTy;
3210+
Id baseTy = accessChain.descHeapInfo.descHeapBaseTy;
32273211
uint32_t explicitArrayStride = accessChain.descHeapInfo.descHeapBaseArrayStride;
32283212
std::vector<Id>& offsets = accessChain.indexChain;
32293213
uint32_t firstArrIndex = accessChain.descHeapInfo.structRsrcTyFirstArrIndex;
32303214
// both typeBufferEXT and UntypedPointer only contains storage class info.
32313215
StorageClass storageClass = (StorageClass)accessChain.descHeapInfo.descHeapStorageClass;
3232-
Id resultTy = makeUntypedPointer(storageClass == spv::StorageClass::StorageBuffer ? spv::StorageClass::StorageBuffer
3233-
: spv::StorageClass::Uniform);
3234-
32353216
// Make the untyped access chain instruction
32363217
Instruction* chain = new Instruction(getUniqueId(), makeUntypedPointer(getStorageClass(base)), Op::OpUntypedAccessChainKHR);
32373218

@@ -3269,21 +3250,38 @@ Id Builder::createDescHeapAccessChain()
32693250
offsets.pop_back();
32703251
addInstruction(std::unique_ptr<Instruction>(chain));
32713252
// Create OpBufferPointer for loading target buffer descriptor.
3272-
Instruction* bufferUntypedDataPtr = new Instruction(getUniqueId(), resultTy, Op::OpBufferPointerEXT);
3273-
bufferUntypedDataPtr->addIdOperand(chain->getResultId());
3274-
addInstruction(std::unique_ptr<Instruction>(bufferUntypedDataPtr));
3275-
// Final/Second untyped access chain loading will be created during loading, current results only
3276-
// refer to the loading 'base'.
3277-
return bufferUntypedDataPtr->getResultId();
3253+
Id bufferPtrTy = makePointer(storageClass, baseTy);
3254+
Instruction* bufferDataPtr = new Instruction(getUniqueId(), bufferPtrTy, Op::OpBufferPointerEXT);
3255+
bufferDataPtr->addIdOperand(chain->getResultId());
3256+
addInstruction(std::unique_ptr<Instruction>(bufferDataPtr));
3257+
3258+
// Form a second, typed access chain for accessing buffer data.
3259+
Id resultTy = baseTy;
3260+
for (int i = 0; i < (int)offsets.size(); ++i) {
3261+
if (isStructType(resultTy)) {
3262+
assert(isConstantScalar(offsets[i]));
3263+
resultTy = getContainedTypeId(resultTy, getConstantScalar(offsets[i]));
3264+
} else
3265+
resultTy = getContainedTypeId(resultTy, offsets[i]);
3266+
}
3267+
resultTy = makePointer(storageClass, resultTy);
3268+
3269+
Instruction* bufferChain = new Instruction(getUniqueId(), resultTy, Op::OpAccessChain);
3270+
bufferChain->reserveOperands(offsets.size() + 1);
3271+
bufferChain->addIdOperand(bufferDataPtr->getResultId());
3272+
for (int i = 0; i < (int)offsets.size(); ++i)
3273+
bufferChain->addIdOperand(offsets[i]);
3274+
addInstruction(std::unique_ptr<Instruction>(bufferChain));
3275+
return bufferChain->getResultId();
32783276
} else {
32793277
// image/sampler heap
32803278
Id strideId = NoResult;
32813279
if (explicitArrayStride == 0) {
3282-
strideId = createConstantSizeOfEXT(untypedResultTy);
3280+
strideId = createConstantSizeOfEXT(baseTy);
32833281
} else {
32843282
strideId = makeUintConstant(explicitArrayStride);
32853283
}
3286-
Id runtimeArrTy = makeRuntimeArray(untypedResultTy);
3284+
Id runtimeArrTy = makeRuntimeArray(baseTy);
32873285
addDecorationId(runtimeArrTy, spv::Decoration::ArrayStrideIdEXT, strideId);
32883286
chain->addIdOperand(runtimeArrTy);
32893287
chain->addIdOperand(base);
Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
spv.descriptorHeap.AtomicImage.comp
22
// Module Version 10400
33
// Generated by (magic number): 8000b
4-
// Id's are bound by 22
4+
// Id's are bound by 21
55

66
Capability Shader
77
Capability Image1D
@@ -18,26 +18,25 @@ spv.descriptorHeap.AtomicImage.comp
1818
Name 4 "main"
1919
Name 7 "resource_heap"
2020
Decorate 7(resource_heap) BuiltIn ResourceHeapEXT
21-
DecorateId 14 DecorationArrayStrideIdEXT 13
21+
DecorateId 13 DecorationArrayStrideIdEXT 12
2222
2: TypeVoid
2323
3: TypeFunction 2
2424
6: TypeUntypedPointerKHR UniformConstant
2525
7(resource_heap): 6(ptr) UntypedVariableKHR UniformConstant
2626
8: TypeInt 32 1
2727
9: 8(int) Constant 1
2828
10: TypeImage 8(int) 1D nonsampled format:R32i
29-
11: TypeUntypedPointerKHR Uniform
30-
13: 8(int) ConstantSizeOfEXT 10
31-
14: TypeRuntimeArray 10
32-
15: TypeInt 32 0
33-
16: 15(int) Constant 0
34-
17: TypePointer Image 8(int)
35-
18: TypeUntypedPointerKHR Image
36-
20: 15(int) Constant 1
29+
12: 8(int) ConstantSizeOfEXT 10
30+
13: TypeRuntimeArray 10
31+
14: TypeInt 32 0
32+
15: 14(int) Constant 0
33+
16: TypePointer Image 8(int)
34+
17: TypeUntypedPointerKHR Image
35+
19: 14(int) Constant 1
3736
4(main): 2 Function None 3
3837
5: Label
39-
12: 6(ptr) UntypedAccessChainKHR 14 7(resource_heap) 9
40-
19: 18(ptr) UntypedImageTexelPointerEXT 10 12 9 16
41-
21: 8(int) AtomicIAdd 19 20 16 9
38+
11: 6(ptr) UntypedAccessChainKHR 13 7(resource_heap) 9
39+
18: 17(ptr) UntypedImageTexelPointerEXT 10 11 9 15
40+
20: 8(int) AtomicIAdd 18 19 15 9
4241
Return
4342
FunctionEnd

Test/baseResults/spv.descriptorHeap.Buffer.comp.out

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
spv.descriptorHeap.Buffer.comp
22
// Module Version 10400
33
// Generated by (magic number): 8000b
4-
// Id's are bound by 30
4+
// Id's are bound by 32
55

66
Capability Shader
77
Capability UntypedPointersKHR
@@ -25,7 +25,7 @@ spv.descriptorHeap.Buffer.comp
2525
MemberDecorate 11(O) 0 Offset 0
2626
Decorate 14(U) Block
2727
MemberDecorate 14(U) 0 Offset 0
28-
DecorateId 19 DecorationArrayStrideIdEXT 18
28+
DecorateId 18 DecorationArrayStrideIdEXT 17
2929
DecorateId 27 DecorationArrayStrideIdEXT 26
3030
2: TypeVoid
3131
3: TypeFunction 2
@@ -38,23 +38,25 @@ spv.descriptorHeap.Buffer.comp
3838
12: 8(int) Constant 0
3939
13: 8(int) Constant 9
4040
14(U): TypeStruct 10(int)
41-
15: TypeUntypedPointerKHR Uniform
42-
17: TypeBufferEXT Uniform
43-
18: 8(int) ConstantSizeOfEXT 17
44-
19: TypeRuntimeArray 17
45-
23: TypeUntypedPointerKHR StorageBuffer
41+
16: TypeBufferEXT Uniform
42+
17: 8(int) ConstantSizeOfEXT 16
43+
18: TypeRuntimeArray 16
44+
19: TypePointer Uniform 14(U)
45+
21: TypePointer Uniform 10(int)
4646
25: TypeBufferEXT StorageBuffer
4747
26: 8(int) ConstantSizeOfEXT 25
4848
27: TypeRuntimeArray 25
49+
28: TypePointer StorageBuffer 11(O)
50+
30: TypePointer StorageBuffer 10(int)
4951
4(main): 2 Function None 3
5052
5: Label
51-
16: 6(ptr) UntypedAccessChainKHR 19 7(resource_heap) 13
52-
20: 15(ptr) BufferPointerEXT 16
53-
21: 15(ptr) UntypedAccessChainKHR 14(U) 20 12
54-
22: 10(int) Load 21
53+
15: 6(ptr) UntypedAccessChainKHR 18 7(resource_heap) 13
54+
20: 19(ptr) BufferPointerEXT 15
55+
22: 21(ptr) AccessChain 20 12
56+
23: 10(int) Load 22
5557
24: 6(ptr) UntypedAccessChainKHR 27 7(resource_heap) 9
56-
28: 23(ptr) BufferPointerEXT 24
57-
29: 23(ptr) UntypedAccessChainKHR 11(O) 28 12
58-
Store 29 22
58+
29: 28(ptr) BufferPointerEXT 24
59+
31: 30(ptr) AccessChain 29 12
60+
Store 31 23
5961
Return
6062
FunctionEnd

Test/baseResults/spv.descriptorHeap.PushConstant.comp.out

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
spv.descriptorHeap.PushConstant.comp
22
// Module Version 10400
33
// Generated by (magic number): 8000b
4-
// Id's are bound by 47
4+
// Id's are bound by 48
55

66
Capability Shader
77
Capability UntypedPointersKHR
@@ -31,8 +31,8 @@ spv.descriptorHeap.PushConstant.comp
3131
Decorate 25 ArrayStride 4
3232
Decorate 26(P) Block
3333
MemberDecorate 26(P) 0 Offset 0
34-
DecorateId 38 DecorationArrayStrideIdEXT 37
35-
Decorate 46 BuiltIn WorkgroupSize
34+
DecorateId 37 DecorationArrayStrideIdEXT 36
35+
Decorate 47 BuiltIn WorkgroupSize
3636
2: TypeVoid
3737
3: TypeFunction 2
3838
6: TypeInt 32 1
@@ -50,14 +50,15 @@ spv.descriptorHeap.PushConstant.comp
5050
27: TypePointer PushConstant 26(P)
5151
28: 27(ptr) Variable PushConstant
5252
30: TypePointer PushConstant 6(int)
53-
34: TypeUntypedPointerKHR StorageBuffer
54-
36: TypeBufferEXT StorageBuffer
55-
37: 6(int) ConstantSizeOfEXT 36
56-
38: TypeRuntimeArray 36
57-
42: 6(int) Constant 1
58-
44: TypeVector 21(int) 3
59-
45: 21(int) Constant 1
60-
46: 44(ivec3) ConstantComposite 45 45 45
53+
35: TypeBufferEXT StorageBuffer
54+
36: 6(int) ConstantSizeOfEXT 35
55+
37: TypeRuntimeArray 35
56+
38: TypePointer StorageBuffer 23(O)
57+
40: TypePointer StorageBuffer 21(int)
58+
43: 6(int) Constant 1
59+
45: TypeVector 21(int) 3
60+
46: 21(int) Constant 1
61+
47: 45(ivec3) ConstantComposite 46 46 46
6162
4(main): 2 Function None 3
6263
5: Label
6364
8(i): 7(ptr) Variable Function
@@ -76,15 +77,15 @@ spv.descriptorHeap.PushConstant.comp
7677
31: 30(ptr) AccessChain 28 9 29
7778
32: 6(int) Load 31
7879
33: 21(int) Bitcast 32
79-
35: 19(ptr) UntypedAccessChainKHR 38 20(resource_heap) 9
80-
39: 34(ptr) BufferPointerEXT 35
81-
40: 34(ptr) UntypedAccessChainKHR 23(O) 39 9 24
82-
Store 40 33
80+
34: 19(ptr) UntypedAccessChainKHR 37 20(resource_heap) 9
81+
39: 38(ptr) BufferPointerEXT 34
82+
41: 40(ptr) AccessChain 39 9 24
83+
Store 41 33
8384
Branch 13
8485
13: Label
85-
41: 6(int) Load 8(i)
86-
43: 6(int) IAdd 41 42
87-
Store 8(i) 43
86+
42: 6(int) Load 8(i)
87+
44: 6(int) IAdd 42 43
88+
Store 8(i) 44
8889
Branch 10
8990
12: Label
9091
Return

0 commit comments

Comments
 (0)