Skip to content

Commit 4c23c9b

Browse files
cdavis5ecdavis5e
committed
MSL: Also handle signedness mismatches in fragment outputs.
It turns out that the number of components is not the only place where Vulkan allows differences but Metal insists on strict equivalence. Vulkan _also_ allows the fragment shader and the attachment to differ in signedness--for example, rendering to an `R8G8B8A8_UINT` image where the fragment shader declares its output to be an `ivec4`. To fix this, we must adjust the `[[color]]` output in the interface block to match the render target. Use the shader output specification mechanism to do this; also move the fragment component count here to save some memory at run time. Signed-off-by: Chip Davis <[email protected]>
1 parent 9770957 commit 4c23c9b

File tree

9 files changed

+549
-149
lines changed

9 files changed

+549
-149
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ set(spirv-cross-util-sources
255255
${CMAKE_CURRENT_SOURCE_DIR}/spirv_cross_util.hpp)
256256

257257
set(spirv-cross-abi-major 0)
258-
set(spirv-cross-abi-minor 67)
258+
set(spirv-cross-abi-minor 68)
259259
set(spirv-cross-abi-patch 0)
260260
set(SPIRV_CROSS_VERSION ${spirv-cross-abi-major}.${spirv-cross-abi-minor}.${spirv-cross-abi-patch})
261261

‎main.cpp‎

Lines changed: 130 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -920,17 +920,31 @@ static void print_help_msl()
920920
"'primitive', or 'patch' to indicate a per-vertex, per-primitive, or per-patch variable.\n"
921921
"\t\tUseful if shader stage interfaces don't match up, as pipeline creation might otherwise fail.\n"
922922
"\t[--msl-add-shader-output <index> <format> <size> <rate>]:\n\t\tSpecify the format of the shader output at <index>.\n"
923-
"\t\t<format> can be 'any32', 'any16', 'u16', 'u8', or 'other', to indicate a 32-bit opaque value, 16-bit opaque value, 16-bit unsigned integer, 8-bit unsigned integer, "
924-
"or other-typed variable. <size> is the vector length of the variable, which must be greater than or equal to that declared in the shader. <rate> can be 'vertex', "
925-
"'primitive', or 'patch' to indicate a per-vertex, per-primitive, or per-patch variable.\n"
926-
"\t\tUseful if shader stage interfaces don't match up, as pipeline creation might otherwise fail.\n"
923+
"\t\t<format> can be 'any32', 'any16', 'u32' 'u16', 'u8', 's32',\n"
924+
"\t\t's16', 's8', or 'other', to indicate a 32-bit opaque value, 16-\n"
925+
"\t\tbit opaque value, 32-bit unsigned integer, 16-bit unsigned\n"
926+
"\t\tinteger, 8-bit unsigned integer, 32-bit signed integer, 16-bit\n"
927+
"\t\tsigned integer, 8-bit signed integer, or other-typed variable.\n"
928+
"\t\t<size> is the vector length of the variable, which must be\n"
929+
"\t\tgreater than or equal to that declared in the shader. <rate> can\n"
930+
"\t\tbe 'vertex', 'primitive', or 'patch' to indicate a per-vertex,\n"
931+
"\t\tper-primitive, or per-patch variable.\n"
932+
"\t\tUseful if shader stage interfaces don't match up, as pipeline\n"
933+
"\t\tcreation might otherwise fail.\n"
927934
"\t[--msl-shader-input <index> <format> <size>]:\n\t\tSpecify the format of the shader input at <index>.\n"
928935
"\t\t<format> can be 'any32', 'any16', 'u16', 'u8', or 'other', to indicate a 32-bit opaque value, 16-bit opaque value, 16-bit unsigned integer, 8-bit unsigned integer, "
929936
"or other-typed variable. <size> is the vector length of the variable, which must be greater than or equal to that declared in the shader."
930937
"\t\tEquivalent to --msl-add-shader-input with a rate of 'vertex'.\n"
931938
"\t[--msl-shader-output <index> <format> <size>]:\n\t\tSpecify the format of the shader output at <index>.\n"
932-
"\t\t<format> can be 'any32', 'any16', 'u16', 'u8', or 'other', to indicate a 32-bit opaque value, 16-bit opaque value, 16-bit unsigned integer, 8-bit unsigned integer, "
933-
"or other-typed variable. <size> is the vector length of the variable, which must be greater than or equal to that declared in the shader."
939+
"\t\t<format> can be 'any32', 'any16', 'u32' 'u16', 'u8', 's32',\n"
940+
"\t\t's16', 's8', or 'other', to indicate a 32-bit opaque value, 16-\n"
941+
"\t\tbit opaque value, 32-bit unsigned integer, 16-bit unsigned\n"
942+
"\t\tinteger, 8-bit unsigned integer, 32-bit signed integer, 16-bit\n"
943+
"\t\tsigned integer, 8-bit signed integer, or other-typed variable.\n"
944+
"\t\t<size> is the vector length of the variable, which must be\n"
945+
"\t\tgreater than or equal to that declared in the shader. <rate> can\n"
946+
"\t\tbe 'vertex', 'primitive', or 'patch' to indicate a per-vertex,\n"
947+
"\t\tper-primitive, or per-patch variable.\n"
934948
"\t\tEquivalent to --msl-add-shader-output with a rate of 'vertex'.\n"
935949
"\t[--msl-raw-buffer-tese-input]:\n\t\tUse raw buffers for tessellation evaluation input.\n"
936950
"\t\tThis allows the use of nested structures and arrays.\n"
@@ -1724,92 +1738,116 @@ static int main_inner(int argc, char *argv[])
17241738
[&args](CLIParser &parser) { args.msl_enable_frag_output_mask = parser.next_hex_uint(); });
17251739
cbs.add("--msl-no-clip-distance-user-varying",
17261740
[&args](CLIParser &) { args.msl_enable_clip_distance_user_varying = false; });
1727-
cbs.add("--msl-add-shader-input", [&args](CLIParser &parser) {
1728-
MSLShaderInterfaceVariable input;
1729-
// Make sure next_uint() is called in-order.
1730-
input.location = parser.next_uint();
1731-
const char *format = parser.next_value_string("other");
1732-
if (strcmp(format, "any32") == 0)
1733-
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1734-
else if (strcmp(format, "any16") == 0)
1735-
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1736-
else if (strcmp(format, "u16") == 0)
1737-
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1738-
else if (strcmp(format, "u8") == 0)
1739-
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1740-
else
1741-
input.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1742-
input.vecsize = parser.next_uint();
1743-
const char *rate = parser.next_value_string("vertex");
1744-
if (strcmp(rate, "primitive") == 0)
1745-
input.rate = MSL_SHADER_VARIABLE_RATE_PER_PRIMITIVE;
1746-
else if (strcmp(rate, "patch") == 0)
1747-
input.rate = MSL_SHADER_VARIABLE_RATE_PER_PATCH;
1748-
else
1749-
input.rate = MSL_SHADER_VARIABLE_RATE_PER_VERTEX;
1750-
args.msl_shader_inputs.push_back(input);
1751-
});
1752-
cbs.add("--msl-add-shader-output", [&args](CLIParser &parser) {
1753-
MSLShaderInterfaceVariable output;
1754-
// Make sure next_uint() is called in-order.
1755-
output.location = parser.next_uint();
1756-
const char *format = parser.next_value_string("other");
1757-
if (strcmp(format, "any32") == 0)
1758-
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1759-
else if (strcmp(format, "any16") == 0)
1760-
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1761-
else if (strcmp(format, "u16") == 0)
1762-
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1763-
else if (strcmp(format, "u8") == 0)
1764-
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1765-
else
1766-
output.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1767-
output.vecsize = parser.next_uint();
1768-
const char *rate = parser.next_value_string("vertex");
1769-
if (strcmp(rate, "primitive") == 0)
1770-
output.rate = MSL_SHADER_VARIABLE_RATE_PER_PRIMITIVE;
1771-
else if (strcmp(rate, "patch") == 0)
1772-
output.rate = MSL_SHADER_VARIABLE_RATE_PER_PATCH;
1773-
else
1774-
output.rate = MSL_SHADER_VARIABLE_RATE_PER_VERTEX;
1775-
args.msl_shader_outputs.push_back(output);
1776-
});
1777-
cbs.add("--msl-shader-input", [&args](CLIParser &parser) {
1778-
MSLShaderInterfaceVariable input;
1779-
// Make sure next_uint() is called in-order.
1780-
input.location = parser.next_uint();
1781-
const char *format = parser.next_value_string("other");
1782-
if (strcmp(format, "any32") == 0)
1783-
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1784-
else if (strcmp(format, "any16") == 0)
1785-
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1786-
else if (strcmp(format, "u16") == 0)
1787-
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1788-
else if (strcmp(format, "u8") == 0)
1789-
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1790-
else
1791-
input.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1792-
input.vecsize = parser.next_uint();
1793-
args.msl_shader_inputs.push_back(input);
1794-
});
1795-
cbs.add("--msl-shader-output", [&args](CLIParser &parser) {
1796-
MSLShaderInterfaceVariable output;
1797-
// Make sure next_uint() is called in-order.
1798-
output.location = parser.next_uint();
1799-
const char *format = parser.next_value_string("other");
1800-
if (strcmp(format, "any32") == 0)
1801-
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1802-
else if (strcmp(format, "any16") == 0)
1803-
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1804-
else if (strcmp(format, "u16") == 0)
1805-
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1806-
else if (strcmp(format, "u8") == 0)
1807-
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1808-
else
1809-
output.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1810-
output.vecsize = parser.next_uint();
1811-
args.msl_shader_outputs.push_back(output);
1812-
});
1741+
cbs.add("--msl-add-shader-input",
1742+
[&args](CLIParser &parser)
1743+
{
1744+
MSLShaderInterfaceVariable input;
1745+
// Make sure next_uint() is called in-order.
1746+
input.location = parser.next_uint();
1747+
const char *format = parser.next_value_string("other");
1748+
if (strcmp(format, "any32") == 0)
1749+
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1750+
else if (strcmp(format, "any16") == 0)
1751+
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1752+
else if (strcmp(format, "u16") == 0)
1753+
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1754+
else if (strcmp(format, "u8") == 0)
1755+
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1756+
else
1757+
input.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1758+
input.vecsize = parser.next_uint();
1759+
const char *rate = parser.next_value_string("vertex");
1760+
if (strcmp(rate, "primitive") == 0)
1761+
input.rate = MSL_SHADER_VARIABLE_RATE_PER_PRIMITIVE;
1762+
else if (strcmp(rate, "patch") == 0)
1763+
input.rate = MSL_SHADER_VARIABLE_RATE_PER_PATCH;
1764+
else
1765+
input.rate = MSL_SHADER_VARIABLE_RATE_PER_VERTEX;
1766+
args.msl_shader_inputs.push_back(input);
1767+
});
1768+
cbs.add("--msl-add-shader-output",
1769+
[&args](CLIParser &parser)
1770+
{
1771+
MSLShaderInterfaceVariable output;
1772+
// Make sure next_uint() is called in-order.
1773+
output.location = parser.next_uint();
1774+
const char *format = parser.next_value_string("other");
1775+
if (strcmp(format, "any32") == 0)
1776+
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1777+
else if (strcmp(format, "any16") == 0)
1778+
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1779+
else if (strcmp(format, "u32") == 0)
1780+
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT32;
1781+
else if (strcmp(format, "u16") == 0)
1782+
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1783+
else if (strcmp(format, "u8") == 0)
1784+
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1785+
else if (strcmp(format, "s32") == 0)
1786+
output.format = MSL_SHADER_VARIABLE_FORMAT_SINT32;
1787+
else if (strcmp(format, "s16") == 0)
1788+
output.format = MSL_SHADER_VARIABLE_FORMAT_SINT16;
1789+
else if (strcmp(format, "s8") == 0)
1790+
output.format = MSL_SHADER_VARIABLE_FORMAT_SINT8;
1791+
else
1792+
output.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1793+
output.vecsize = parser.next_uint();
1794+
const char *rate = parser.next_value_string("vertex");
1795+
if (strcmp(rate, "primitive") == 0)
1796+
output.rate = MSL_SHADER_VARIABLE_RATE_PER_PRIMITIVE;
1797+
else if (strcmp(rate, "patch") == 0)
1798+
output.rate = MSL_SHADER_VARIABLE_RATE_PER_PATCH;
1799+
else
1800+
output.rate = MSL_SHADER_VARIABLE_RATE_PER_VERTEX;
1801+
args.msl_shader_outputs.push_back(output);
1802+
});
1803+
cbs.add("--msl-shader-input",
1804+
[&args](CLIParser &parser)
1805+
{
1806+
MSLShaderInterfaceVariable input;
1807+
// Make sure next_uint() is called in-order.
1808+
input.location = parser.next_uint();
1809+
const char *format = parser.next_value_string("other");
1810+
if (strcmp(format, "any32") == 0)
1811+
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1812+
else if (strcmp(format, "any16") == 0)
1813+
input.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1814+
else if (strcmp(format, "u16") == 0)
1815+
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1816+
else if (strcmp(format, "u8") == 0)
1817+
input.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1818+
else
1819+
input.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1820+
input.vecsize = parser.next_uint();
1821+
args.msl_shader_inputs.push_back(input);
1822+
});
1823+
cbs.add("--msl-shader-output",
1824+
[&args](CLIParser &parser)
1825+
{
1826+
MSLShaderInterfaceVariable output;
1827+
// Make sure next_uint() is called in-order.
1828+
output.location = parser.next_uint();
1829+
const char *format = parser.next_value_string("other");
1830+
if (strcmp(format, "any32") == 0)
1831+
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY32;
1832+
else if (strcmp(format, "any16") == 0)
1833+
output.format = MSL_SHADER_VARIABLE_FORMAT_ANY16;
1834+
else if (strcmp(format, "u32") == 0)
1835+
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT32;
1836+
else if (strcmp(format, "u16") == 0)
1837+
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT16;
1838+
else if (strcmp(format, "u8") == 0)
1839+
output.format = MSL_SHADER_VARIABLE_FORMAT_UINT8;
1840+
else if (strcmp(format, "s32") == 0)
1841+
output.format = MSL_SHADER_VARIABLE_FORMAT_SINT32;
1842+
else if (strcmp(format, "s16") == 0)
1843+
output.format = MSL_SHADER_VARIABLE_FORMAT_SINT16;
1844+
else if (strcmp(format, "s8") == 0)
1845+
output.format = MSL_SHADER_VARIABLE_FORMAT_SINT8;
1846+
else
1847+
output.format = MSL_SHADER_VARIABLE_FORMAT_OTHER;
1848+
output.vecsize = parser.next_uint();
1849+
args.msl_shader_outputs.push_back(output);
1850+
});
18131851
cbs.add("--msl-raw-buffer-tese-input", [&args](CLIParser &) { args.msl_raw_buffer_tese_input = true; });
18141852
cbs.add("--msl-multi-patch-workgroup", [&args](CLIParser &) { args.msl_multi_patch_workgroup = true; });
18151853
cbs.add("--msl-vertex-for-tessellation", [&args](CLIParser &) { args.msl_vertex_for_tessellation = true; });

‎reference/opt/shaders-msl/frag/fragment.signed-mismatch.frag‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
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 main0_out
48+
{
49+
uint4 out0 [[color(0)]];
50+
ushort4 out1_0 [[color(1)]];
51+
ushort3 out1_1 [[color(2)]];
52+
int4 out3 [[color(3)]];
53+
short2 out4 [[color(4)]];
54+
short4 m_location_5 [[color(5)]];
55+
float4 out6 [[color(6)]];
56+
};
57+
58+
struct main0_in
59+
{
60+
uchar4 vColor [[user(locn0)]];
61+
};
62+
63+
fragment main0_out main0(main0_in in [[stage_in]])
64+
{
65+
main0_out out = {};
66+
int4 out0 = {};
67+
spvUnsafeArray<short3, 2> out1 = {};
68+
uint2 out3 = {};
69+
ushort out4 = {};
70+
uint2 out5 = {};
71+
out0 = int4(uint4(in.vColor));
72+
out1[0] = short3(ushort3(in.vColor.yyy));
73+
out1[1] = short3(ushort3(in.vColor.wxz));
74+
out3.x = uint(in.vColor.x);
75+
out3.y = uint(in.vColor.w);
76+
out4 = ushort(in.vColor.z);
77+
out5 = uint2(in.vColor.zx);
78+
out.out6 = float4(in.vColor);
79+
out.out0 = uint4(out0);
80+
out.out1_0 = ushort4(out1[0].xyzz);
81+
out.out1_1 = ushort3(out1[1]);
82+
out.out3.xy = int2(out3);
83+
out.out4.x = short(out4);
84+
out.m_location_5.zw = short2(out5);
85+
return out;
86+
}
87+

0 commit comments

Comments
 (0)