Merge pull request #2439 from KhronosGroup/fix-2261

MSL: Add option to auto-disable rasterization based on Position usage

CMakeLists.txt

@@ -246,7 +246,7 @@ set(spirv-cross-util-sources
 		${CMAKE_CURRENT_SOURCE_DIR}/spirv_cross_util.hpp)
 
 set(spirv-cross-abi-major 0)
-set(spirv-cross-abi-minor 64)
+set(spirv-cross-abi-minor 65)
 set(spirv-cross-abi-patch 0)
 set(SPIRV_CROSS_VERSION ${spirv-cross-abi-major}.${spirv-cross-abi-minor}.${spirv-cross-abi-patch})
 

main.cpp

@@ -682,6 +682,8 @@ struct CLIArguments
 	bool msl_readwrite_texture_fences = true;
 	bool msl_agx_manual_cube_grad_fixup = false;
 	bool msl_input_attachment_is_ds_attachment = false;
+	bool msl_disable_rasterization = false;
+	bool msl_auto_disable_rasterization = false;
 	const char *msl_combined_sampler_suffix = nullptr;
 	bool glsl_emit_push_constant_as_ubo = false;
 	bool glsl_emit_ubo_as_plain_uniforms = false;
@@ -983,7 +985,9 @@ static void print_help_msl()
 	                "\t\tAll released Apple Silicon GPUs to date ignore one of the three partial derivatives\n"
 	                "\t\tbased on the selected major axis, and expect the remaining derivatives to be\n"
 	                "\t\tpartially transformed. This fixup gives correct results on Apple Silicon.\n"
-	                "\t[--msl-combined-sampler-suffix <suffix>]:\n\t\tUses a custom suffix for combined samplers.\n");
+	                "\t[--msl-combined-sampler-suffix <suffix>]:\n\t\tUses a custom suffix for combined samplers.\n"
+	                "\t[--msl-disable-rasterization]:\n\t\tDisables rasterization and returns void from vertex-like entry points.\n"
+	                "\t[--msl-auto-disable-rasterization]:\n\t\tDisables rasterization if BuiltInPosition is not written.\n");
 	// clang-format on
 }
 
@@ -1265,6 +1269,8 @@ static string compile_iteration(const CLIArguments &args, std::vector<uint32_t>
 		msl_opts.input_attachment_is_ds_attachment = args.msl_input_attachment_is_ds_attachment;
 		msl_opts.readwrite_texture_fences = args.msl_readwrite_texture_fences;
 		msl_opts.agx_manual_cube_grad_fixup = args.msl_agx_manual_cube_grad_fixup;
+		msl_opts.disable_rasterization = args.msl_disable_rasterization;
+		msl_opts.auto_disable_rasterization = args.msl_auto_disable_rasterization;
 		msl_comp->set_msl_options(msl_opts);
 		for (auto &v : args.msl_discrete_descriptor_sets)
 			msl_comp->add_discrete_descriptor_set(v);
@@ -1830,6 +1836,8 @@ static int main_inner(int argc, char *argv[])
 	cbs.add("--msl-replace-recursive-inputs",
 	        [&args](CLIParser &) { args.msl_replace_recursive_inputs = true; });
 	cbs.add("--msl-input-attachment-is-ds-attachment", [&args](CLIParser &) { args.msl_input_attachment_is_ds_attachment = true; });
+	cbs.add("--msl-disable-rasterization", [&args](CLIParser &) { args.msl_disable_rasterization = true; });
+	cbs.add("--msl-auto-disable-rasterization", [&args](CLIParser &) { args.msl_auto_disable_rasterization = true; });
 	cbs.add("--extension", [&args](CLIParser &parser) { args.extensions.push_back(parser.next_string()); });
 	cbs.add("--rename-entry-point", [&args](CLIParser &parser) {
 		auto old_name = parser.next_string();

reference/opt/shaders-msl/asm/frag/unreachable-return.msl23.spv14.asm.frag

@@ -0,0 +1,25 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct buff_t
+{
+    int m0[1024];
+};
+
+struct main0_out
+{
+    float4 frag_clr [[color(0)]];
+};
+
+fragment main0_out main0(device buff_t& buff [[buffer(0)]], float4 gl_FragCoord [[position]])
+{
+    main0_out out = {};
+    int4 _16 = int4(gl_FragCoord);
+    out.frag_clr = float4(0.0, 0.0, 1.0, 1.0);
+    buff.m0[(_16.y * 32) + _16.x] = 1;
+    discard_fragment();
+    return out;
+}
+

reference/shaders-msl-no-opt/vert/no-position-write.auto-disable-rasterization.msl21.vert

@@ -0,0 +1,16 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_out
+{
+    int o_value [[user(locn0)]];
+};
+
+vertex void main0()
+{
+    main0_out out = {};
+    out.o_value = 10;
+}
+

reference/shaders-msl-no-opt/vert/no-position-write.disable-rasterization.msl21.vert

@@ -0,0 +1,18 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct main0_out
+{
+    int o_value [[user(locn0)]];
+    float4 gl_Position [[position, invariant]];
+};
+
+vertex void main0()
+{
+    main0_out out = {};
+    out.o_value = 10;
+    out.gl_Position = float4(10.0);
+}
+

shaders-msl-no-opt/vert/no-position-write.auto-disable-rasterization.msl21.vert

@@ -0,0 +1,13 @@
+#version 450
+
+out gl_PerVertex
+{
+	invariant vec4 gl_Position;
+};
+
+layout(location = 0) out int o_value;
+
+void main()
+{
+   o_value = 10;
+}

shaders-msl-no-opt/vert/no-position-write.disable-rasterization.msl21.vert

@@ -0,0 +1,14 @@
+#version 450
+
+out gl_PerVertex
+{
+	invariant vec4 gl_Position;
+};
+
+layout(location = 0) out int o_value;
+
+void main()
+{
+   o_value = 10;
+   gl_Position = vec4(10.0);
+}

spirv_cross_c.cpp

@@ -569,6 +569,10 @@ spvc_result spvc_compiler_options_set_uint(spvc_compiler_options options, spvc_c
 		options->msl.disable_rasterization = value != 0;
 		break;
 
+	case SPVC_COMPILER_OPTION_MSL_AUTO_DISABLE_RASTERIZATION:
+		options->msl.auto_disable_rasterization = value != 0;
+		break;
+
 	case SPVC_COMPILER_OPTION_MSL_CAPTURE_OUTPUT_TO_BUFFER:
 		options->msl.capture_output_to_buffer = value != 0;
 		break;

spirv_cross_c.h

@@ -40,7 +40,7 @@ extern "C" {
 /* Bumped if ABI or API breaks backwards compatibility. */
 #define SPVC_C_API_VERSION_MAJOR 0
 /* Bumped if APIs or enumerations are added in a backwards compatible way. */
-#define SPVC_C_API_VERSION_MINOR 64
+#define SPVC_C_API_VERSION_MINOR 65
 /* Bumped if internal implementation details change. */
 #define SPVC_C_API_VERSION_PATCH 0
 
@@ -748,6 +748,8 @@ typedef enum spvc_compiler_option
 	SPVC_COMPILER_OPTION_HLSL_USE_ENTRY_POINT_NAME = 90 | SPVC_COMPILER_OPTION_HLSL_BIT,
 	SPVC_COMPILER_OPTION_HLSL_PRESERVE_STRUCTURED_BUFFERS = 91 | SPVC_COMPILER_OPTION_HLSL_BIT,
 
+	SPVC_COMPILER_OPTION_MSL_AUTO_DISABLE_RASTERIZATION = 92 | SPVC_COMPILER_OPTION_MSL_BIT,
+
 	SPVC_COMPILER_OPTION_INT_MAX = 0x7fffffff
 } spvc_compiler_option;
 

spirv_msl.cpp

@@ -1003,6 +1003,7 @@ void CompilerMSL::build_implicit_builtins()
 	// If we're returning a struct from a vertex-like entry point, we must return a position attribute.
 	bool need_position = (get_execution_model() == ExecutionModelVertex || is_tese_shader()) &&
 	                     !capture_output_to_buffer && !get_is_rasterization_disabled() &&
+	                     !msl_options.auto_disable_rasterization &&
 	                     !active_output_builtins.get(BuiltInPosition);
 
 	if (need_position)
@@ -1039,6 +1040,10 @@ void CompilerMSL::build_implicit_builtins()
 		});
 		need_position = has_output && !active_output_builtins.get(BuiltInPosition);
 	}
+	else if (!active_output_builtins.get(BuiltInPosition) && msl_options.auto_disable_rasterization)
+	{
+		is_rasterization_disabled = true;
+	}
 
 	if (need_position)
 	{

spirv_msl.hpp

@@ -536,6 +536,10 @@ class CompilerMSL : public CompilerGLSL
 		// if the fragment does not modify the depth value.
 		bool input_attachment_is_ds_attachment = false;
 
+		// If BuiltInPosition is not written, automatically disable rasterization.
+		// The result can be queried with get_is_rasterization_disabled.
+		bool auto_disable_rasterization = false;
+
 		bool is_ios() const
 		{
 			return platform == iOS;

test_shaders.py

@@ -392,6 +392,10 @@ def cross_compile_msl(shader, spirv, opt, iterations, paths):
         msl_args.append('ClipDistance')
     if '.relax-nan.' in shader:
         msl_args.append('--relax-nan-checks')
+    if '.auto-disable-rasterization.' in shader:
+        msl_args.append('--msl-auto-disable-rasterization')
+    if '.disable-rasterization.' in shader:
+        msl_args.append('--msl-disable-rasterization')
 
     subprocess.check_call(msl_args)