Skip to content

Commit 0e2880a

Browse files
HansKristian-WorkHansKristian-Work
authored
Merge pull request #1729 from KhronosGroup/fix-1726
MSL: Consider that function/private variables can be block-like.
2 parents 840d448 + 2eea6a5 commit 0e2880a

3 files changed

Lines changed: 234 additions & 0 deletions

File tree

‎reference/shaders-msl-no-opt/asm/frag/copy-memory-block-like-thread-local.asm.frag‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
#pragma clang diagnostic ignored "-Wmissing-prototypes"
2+
#pragma clang diagnostic ignored "-Wmissing-braces"
3+
4+
#include <metal_stdlib>
5+
#include <simd/simd.h>
6+
7+
using namespace metal;
8+
9+
template<typename T, size_t Num>
10+
struct spvUnsafeArray
11+
{
12+
T elements[Num ? Num : 1];
13+
14+
thread T& operator [] (size_t pos) thread
15+
{
16+
return elements[pos];
17+
}
18+
constexpr const thread T& operator [] (size_t pos) const thread
19+
{
20+
return elements[pos];
21+
}
22+
23+
device T& operator [] (size_t pos) device
24+
{
25+
return elements[pos];
26+
}
27+
constexpr const device T& operator [] (size_t pos) const device
28+
{
29+
return elements[pos];
30+
}
31+
32+
constexpr const constant T& operator [] (size_t pos) const constant
33+
{
34+
return elements[pos];
35+
}
36+
37+
threadgroup T& operator [] (size_t pos) threadgroup
38+
{
39+
return elements[pos];
40+
}
41+
constexpr const threadgroup T& operator [] (size_t pos) const threadgroup
42+
{
43+
return elements[pos];
44+
}
45+
};
46+
47+
struct _3
48+
{
49+
float _m0[4];
50+
};
51+
52+
template<typename T, uint A>
53+
inline void spvArrayCopyFromConstantToStack1(thread T (&dst)[A], constant T (&src)[A])
54+
{
55+
for (uint i = 0; i < A; i++)
56+
{
57+
dst[i] = src[i];
58+
}
59+
}
60+
61+
template<typename T, uint A>
62+
inline void spvArrayCopyFromConstantToThreadGroup1(threadgroup T (&dst)[A], constant T (&src)[A])
63+
{
64+
for (uint i = 0; i < A; i++)
65+
{
66+
dst[i] = src[i];
67+
}
68+
}
69+
70+
template<typename T, uint A>
71+
inline void spvArrayCopyFromStackToStack1(thread T (&dst)[A], thread const T (&src)[A])
72+
{
73+
for (uint i = 0; i < A; i++)
74+
{
75+
dst[i] = src[i];
76+
}
77+
}
78+
79+
template<typename T, uint A>
80+
inline void spvArrayCopyFromStackToThreadGroup1(threadgroup T (&dst)[A], thread const T (&src)[A])
81+
{
82+
for (uint i = 0; i < A; i++)
83+
{
84+
dst[i] = src[i];
85+
}
86+
}
87+
88+
template<typename T, uint A>
89+
inline void spvArrayCopyFromThreadGroupToStack1(thread T (&dst)[A], threadgroup const T (&src)[A])
90+
{
91+
for (uint i = 0; i < A; i++)
92+
{
93+
dst[i] = src[i];
94+
}
95+
}
96+
97+
template<typename T, uint A>
98+
inline void spvArrayCopyFromThreadGroupToThreadGroup1(threadgroup T (&dst)[A], threadgroup const T (&src)[A])
99+
{
100+
for (uint i = 0; i < A; i++)
101+
{
102+
dst[i] = src[i];
103+
}
104+
}
105+
106+
template<typename T, uint A>
107+
inline void spvArrayCopyFromDeviceToDevice1(device T (&dst)[A], device const T (&src)[A])
108+
{
109+
for (uint i = 0; i < A; i++)
110+
{
111+
dst[i] = src[i];
112+
}
113+
}
114+
115+
template<typename T, uint A>
116+
inline void spvArrayCopyFromConstantToDevice1(device T (&dst)[A], constant T (&src)[A])
117+
{
118+
for (uint i = 0; i < A; i++)
119+
{
120+
dst[i] = src[i];
121+
}
122+
}
123+
124+
template<typename T, uint A>
125+
inline void spvArrayCopyFromStackToDevice1(device T (&dst)[A], thread const T (&src)[A])
126+
{
127+
for (uint i = 0; i < A; i++)
128+
{
129+
dst[i] = src[i];
130+
}
131+
}
132+
133+
template<typename T, uint A>
134+
inline void spvArrayCopyFromThreadGroupToDevice1(device T (&dst)[A], threadgroup const T (&src)[A])
135+
{
136+
for (uint i = 0; i < A; i++)
137+
{
138+
dst[i] = src[i];
139+
}
140+
}
141+
142+
template<typename T, uint A>
143+
inline void spvArrayCopyFromDeviceToStack1(thread T (&dst)[A], device const T (&src)[A])
144+
{
145+
for (uint i = 0; i < A; i++)
146+
{
147+
dst[i] = src[i];
148+
}
149+
}
150+
151+
template<typename T, uint A>
152+
inline void spvArrayCopyFromDeviceToThreadGroup1(threadgroup T (&dst)[A], device const T (&src)[A])
153+
{
154+
for (uint i = 0; i < A; i++)
155+
{
156+
dst[i] = src[i];
157+
}
158+
}
159+
160+
fragment void main0()
161+
{
162+
spvUnsafeArray<float, 4> _20;
163+
_20[0u] = 0.0;
164+
_20[1u] = 0.0;
165+
_20[2u] = 0.0;
166+
_20[3u] = 0.0;
167+
_3 _19;
168+
spvArrayCopyFromStackToStack1(_19._m0, _20.elements);
169+
}
170+

‎shaders-msl-no-opt/asm/frag/copy-memory-block-like-thread-local.asm.frag‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
; SPIR-V
2+
; Version: 1.3
3+
; Generator: Google rspirv; 0
4+
; Bound: 43
5+
; Schema: 0
6+
OpCapability ImageQuery
7+
OpCapability Int8
8+
OpCapability RuntimeDescriptorArray
9+
OpCapability StorageImageWriteWithoutFormat
10+
OpCapability Shader
11+
OpCapability VulkanMemoryModel
12+
OpExtension "SPV_EXT_descriptor_indexing"
13+
OpExtension "SPV_KHR_vulkan_memory_model"
14+
OpMemoryModel Logical Vulkan
15+
OpEntryPoint Fragment %1 "main"
16+
OpExecutionMode %1 OriginUpperLeft
17+
OpDecorate %2 ArrayStride 4
18+
OpMemberDecorate %3 0 Offset 0
19+
%4 = OpTypeInt 32 0
20+
%5 = OpTypeFloat 32
21+
%6 = OpTypePointer Function %5
22+
%7 = OpTypeVoid
23+
%8 = OpTypeFunction %7
24+
%9 = OpConstant %4 0
25+
%10 = OpConstant %4 1
26+
%11 = OpConstant %4 2
27+
%12 = OpConstant %4 4
28+
%13 = OpConstant %4 3
29+
%14 = OpConstant %5 0
30+
%2 = OpTypeArray %5 %12
31+
%15 = OpTypePointer Function %2
32+
%16 = OpTypeFunction %7 %15
33+
%3 = OpTypeStruct %2
34+
%17 = OpTypePointer Function %3
35+
%1 = OpFunction %7 None %8
36+
%31 = OpLabel
37+
%33 = OpVariable %17 Function
38+
%34 = OpVariable %15 Function
39+
%39 = OpAccessChain %6 %34 %9
40+
OpStore %39 %14
41+
%40 = OpAccessChain %6 %34 %10
42+
OpStore %40 %14
43+
%41 = OpAccessChain %6 %34 %11
44+
OpStore %41 %14
45+
%42 = OpAccessChain %6 %34 %13
46+
OpStore %42 %14
47+
%37 = OpAccessChain %15 %33 %9
48+
OpCopyMemory %37 %34
49+
OpReturn
50+
OpFunctionEnd

‎spirv_msl.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8511,13 +8511,27 @@ void CompilerMSL::emit_array_copy(const string &lhs, uint32_t lhs_id, uint32_t r
85118511

85128512
// Special considerations for stage IO variables.
85138513
// If the variable is actually backed by non-user visible device storage, we use array templates for those.
8514+
//
8515+
// Another special consideration is given to thread local variables which happen to have Offset decorations
8516+
// applied to them. Block-like types do not use array templates, so we need to force POD path if we detect
8517+
// these scenarios. This check isn't perfect since it would be technically possible to mix and match these things,
8518+
// and for a fully correct solution we might have to track array template state through access chains as well,
8519+
// but for all reasonable use cases, this should suffice.
8520+
// This special case should also only apply to Function/Private storage classes.
8521+
// We should not check backing variable for temporaries.
85148522
auto *lhs_var = maybe_get_backing_variable(lhs_id);
85158523
if (lhs_var && lhs_storage == StorageClassStorageBuffer && storage_class_array_is_thread(lhs_var->storage))
85168524
lhs_is_array_template = true;
8525+
else if (lhs_var && (lhs_storage == StorageClassFunction || lhs_storage == StorageClassPrivate) &&
8526+
type_is_block_like(get<SPIRType>(lhs_var->basetype)))
8527+
lhs_is_array_template = false;
85178528

85188529
auto *rhs_var = maybe_get_backing_variable(rhs_id);
85198530
if (rhs_var && rhs_storage == StorageClassStorageBuffer && storage_class_array_is_thread(rhs_var->storage))
85208531
rhs_is_array_template = true;
8532+
else if (rhs_var && (rhs_storage == StorageClassFunction || rhs_storage == StorageClassPrivate) &&
8533+
type_is_block_like(get<SPIRType>(rhs_var->basetype)))
8534+
rhs_is_array_template = false;
85218535

85228536
// If threadgroup storage qualifiers are *not* used:
85238537
// Avoid spvCopy* wrapper functions; Otherwise, spvUnsafeArray<> template cannot be used with that storage qualifier.

0 commit comments

Comments
 (0)