Add option to enforce fragment execution with side effects in MSL

Metal will incorrectly discard fragments with side effects under
certain circumstances prematurely. The conditions are the following:
 - Fragment will always be discarded
 - Pre fragment depth fails fails
 - Modifies depth value for a constant value in the fragment shader.
   This constant value will also fail the depth test.

However, Metal will also discard the fragment even if it has
operations with side effects inside the fragment shader before the
discard operation.

Vulkan states the graphics pipeline to execute in the following
order:
 - Pre fragment depth test (cannot discard here due to modifying
   depth value in fragment shader)
 - Fragment shader (where the depth is modified)
 - Post fragment depth test (where fragment should be discarded)

Therefore, we need to enforce fragment shader execution and not
let Metal discard the fragment before that for such cases. This
change adds an option to provide such utility.

spirv_cross_c.cpp

@@ -754,6 +754,10 @@ spvc_result spvc_compiler_options_set_uint(spvc_compiler_options options, spvc_c
 	case SPVC_COMPILER_OPTION_MSL_AGX_MANUAL_CUBE_GRAD_FIXUP:
 		options->msl.agx_manual_cube_grad_fixup = value != 0;
 		break;
+
+	case SPVC_COMPILER_OPTION_MSL_ENFORCE_FRAGMENT_WITH_SIDE_EFFECTS_EXECUTION:
+		options->msl.enforce_fragment_with_side_effects_execution = value != 0;
+		break;
 #endif
 
 	default:

spirv_cross_c.h

@@ -728,6 +728,7 @@ typedef enum spvc_compiler_option
 	SPVC_COMPILER_OPTION_MSL_READWRITE_TEXTURE_FENCES = 86 | SPVC_COMPILER_OPTION_MSL_BIT,
 	SPVC_COMPILER_OPTION_MSL_REPLACE_RECURSIVE_INPUTS = 87 | SPVC_COMPILER_OPTION_MSL_BIT,
 	SPVC_COMPILER_OPTION_MSL_AGX_MANUAL_CUBE_GRAD_FIXUP = 88 | SPVC_COMPILER_OPTION_MSL_BIT,
+	SPVC_COMPILER_OPTION_MSL_ENFORCE_FRAGMENT_WITH_SIDE_EFFECTS_EXECUTION = 89 | SPVC_COMPILER_OPTION_MSL_BIT,
 
 	SPVC_COMPILER_OPTION_INT_MAX = 0x7fffffff
 } spvc_compiler_option;

spirv_msl.cpp

@@ -1553,8 +1553,10 @@ string CompilerMSL::compile()
 	if (needs_manual_helper_invocation_updates() &&
 	    (active_input_builtins.get(BuiltInHelperInvocation) || needs_helper_invocation))
 	{
-		string discard_expr =
-		    join(builtin_to_glsl(BuiltInHelperInvocation, StorageClassInput), " = true, discard_fragment()");
+		string builtin_helper_invocation = builtin_to_glsl(BuiltInHelperInvocation, StorageClassInput);
+		string discard_expr = join(builtin_helper_invocation, " = true, discard_fragment()");
+		if (msl_options.enforce_fragment_with_side_effects_execution)
+			discard_expr = join("!", builtin_helper_invocation, " ? (", discard_expr, ") : (void)0");
 		backend.discard_literal = discard_expr;
 		backend.demote_literal = discard_expr;
 	}
@@ -1703,14 +1705,15 @@ void CompilerMSL::preprocess_op_codes()
 
 	// Fragment shaders that both write to storage resources and discard fragments
 	// need checks on the writes, to work around Metal allowing these writes despite
-	// the fragment being dead.
-	if (msl_options.check_discarded_frag_stores && preproc.uses_discard &&
-	    (preproc.uses_buffer_write || preproc.uses_image_write))
+	// the fragment being dead. We also require to force Metal to execute fragment
+	// shaders instead of being prematurely discarded.
+	if (preproc.uses_discard && (preproc.uses_buffer_write || preproc.uses_image_write))
 	{
-		frag_shader_needs_discard_checks = true;
-		needs_helper_invocation = true;
+		bool should_enable = (msl_options.check_discarded_frag_stores || msl_options.enforce_fragment_with_side_effects_execution);
+		frag_shader_needs_discard_checks |= msl_options.check_discarded_frag_stores;
+		needs_helper_invocation |= should_enable;
 		// Fragment discard store checks imply manual HelperInvocation updates.
-		msl_options.manual_helper_invocation_updates = true;
+		msl_options.manual_helper_invocation_updates |= should_enable;
 	}
 
 	if (is_intersection_query())

spirv_msl.hpp

@@ -516,6 +516,19 @@ class CompilerMSL : public CompilerGLSL
 		// transformed.
 		bool agx_manual_cube_grad_fixup = false;
 
+		// Metal will discard fragments with side effects under certain circumstances prematurely.
+		// Example: CTS test dEQP-VK.fragment_operations.early_fragment.discard_no_early_fragment_tests_depth
+		// Test will render a full screen quad with varying depth [0,1] for each fragment.
+		// Each fragment will do an operation with side effects, modify the depth value and
+		// discard the fragment. The test expects the fragment to be run due to:
+		// https://registry.khronos.org/vulkan/specs/1.0-extensions/html/vkspec.html#fragops-shader-depthreplacement
+		// which states that the fragment shader must be run due to replacing the depth in shader.
+		// However, Metal may prematurely discards fragments without executing them
+		// (I believe this to be due to a greedy optimization on their end) making the test fail.
+		// This option enforces fragment execution for such cases where the fragment has operations
+		// with side effects. Provided as an option hoping Metal will fix this issue in the future.
+		bool enforce_fragment_with_side_effects_execution = false;
+
 		bool is_ios() const
 		{
 			return platform == iOS;