Merge pull request #1264 from KhronosGroup/msl-argument-buffer-persist

MSL: Add support for force-activating IAB resources.

Author: HansKristian-Work

CMakeLists.txt

@@ -312,7 +312,7 @@ if (SPIRV_CROSS_STATIC)
 endif()
 
 set(spirv-cross-abi-major 0)
-set(spirv-cross-abi-minor 22)
+set(spirv-cross-abi-minor 23)
 set(spirv-cross-abi-patch 0)
 
 if (SPIRV_CROSS_SHARED)

main.cpp

@@ -521,6 +521,7 @@ struct CLIArguments
 	bool msl_view_index_from_device_index = false;
 	bool msl_dispatch_base = false;
 	bool msl_decoration_binding = false;
+	bool msl_force_active_argument_buffer_resources = false;
 	bool glsl_emit_push_constant_as_ubo = false;
 	bool glsl_emit_ubo_as_plain_uniforms = false;
 	bool vulkan_glsl_disable_ext_samplerless_texture_functions = false;
@@ -612,6 +613,7 @@ static void print_help()
 	                "\t[--msl-dispatch-base]\n"
 	                "\t[--msl-dynamic-buffer <set index> <binding>]\n"
 	                "\t[--msl-decoration-binding]\n"
+	                "\t[--msl-force-active-argument-buffer-resources]\n"
 	                "\t[--hlsl]\n"
 	                "\t[--reflect]\n"
 	                "\t[--shader-model]\n"
@@ -801,6 +803,7 @@ static string compile_iteration(const CLIArguments &args, std::vector<uint32_t>
 		msl_opts.view_index_from_device_index = args.msl_view_index_from_device_index;
 		msl_opts.dispatch_base = args.msl_dispatch_base;
 		msl_opts.enable_decoration_binding = args.msl_decoration_binding;
+		msl_opts.force_active_argument_buffer_resources = args.msl_force_active_argument_buffer_resources;
 		msl_comp->set_msl_options(msl_opts);
 		for (auto &v : args.msl_discrete_descriptor_sets)
 			msl_comp->add_discrete_descriptor_set(v);
@@ -1148,6 +1151,9 @@ static int main_inner(int argc, char *argv[])
 		args.msl_dynamic_buffers.push_back(make_pair(desc_set, binding));
 	});
 	cbs.add("--msl-decoration-binding", [&args](CLIParser &) { args.msl_decoration_binding = true; });
+	cbs.add("--msl-force-active-argument-buffer-resources", [&args](CLIParser &) {
+		args.msl_force_active_argument_buffer_resources = 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/shaders-msl-no-opt/frag/force-active-resources.msl2.argument..force-active.discrete.frag

@@ -0,0 +1,31 @@
+#include <metal_stdlib>
+#include <simd/simd.h>
+
+using namespace metal;
+
+struct spvDescriptorSetBuffer0
+{
+    texture2d<float> uTexture2 [[id(0)]];
+    sampler uTexture2Smplr [[id(1)]];
+    texture2d<float> uTexture1 [[id(2)]];
+    sampler uTexture1Smplr [[id(3)]];
+};
+
+struct main0_out
+{
+    float4 FragColor [[color(0)]];
+};
+
+struct main0_in
+{
+    float2 vUV [[user(locn0)]];
+};
+
+fragment main0_out main0(main0_in in [[stage_in]], constant spvDescriptorSetBuffer0& spvDescriptorSet0 [[buffer(0)]], texture2d<float> uTextureDiscrete2 [[texture(0)]], sampler uTextureDiscrete2Smplr [[sampler(0)]])
+{
+    main0_out out = {};
+    out.FragColor = spvDescriptorSet0.uTexture2.sample(spvDescriptorSet0.uTexture2Smplr, in.vUV);
+    out.FragColor += uTextureDiscrete2.sample(uTextureDiscrete2Smplr, in.vUV);
+    return out;
+}
+

shaders-msl-no-opt/frag/force-active-resources.msl2.argument..force-active.discrete.frag

@@ -0,0 +1,15 @@
+#version 450
+
+layout(location = 0) in vec2 vUV;
+layout(location = 0) out vec4 FragColor;
+
+layout(set = 0, binding = 0) uniform sampler2D uTexture1;
+layout(set = 0, binding = 1) uniform sampler2D uTexture2;
+layout(set = 2, binding = 0) uniform sampler2D uTextureDiscrete1;
+layout(set = 2, binding = 1) uniform sampler2D uTextureDiscrete2;
+
+void main()
+{
+	FragColor = texture(uTexture2, vUV);
+	FragColor += texture(uTextureDiscrete2, vUV);
+}

spirv_cross_c.cpp

@@ -593,6 +593,10 @@ spvc_result spvc_compiler_options_set_uint(spvc_compiler_options options, spvc_c
 	case SPVC_COMPILER_OPTION_MSL_ENABLE_DECORATION_BINDING:
 		options->msl.enable_decoration_binding = value != 0;
 		break;
+
+	case SPVC_COMPILER_OPTION_MSL_FORCE_ACTIVE_ARGUMENT_BUFFER_RESOURCES:
+		options->msl.force_active_argument_buffer_resources = value != 0;
+		break;
 #endif
 
 	default:

spirv_cross_c.h

@@ -33,7 +33,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 22
+#define SPVC_C_API_VERSION_MINOR 23
 /* Bumped if internal implementation details change. */
 #define SPVC_C_API_VERSION_PATCH 0
 
@@ -571,6 +571,7 @@ typedef enum spvc_compiler_option
 	SPVC_COMPILER_OPTION_MSL_INVARIANT_FP_MATH = 47 | SPVC_COMPILER_OPTION_MSL_BIT,
 	SPVC_COMPILER_OPTION_MSL_EMULATE_CUBEMAP_ARRAY = 48 | SPVC_COMPILER_OPTION_MSL_BIT,
 	SPVC_COMPILER_OPTION_MSL_ENABLE_DECORATION_BINDING = 49 | SPVC_COMPILER_OPTION_MSL_BIT,
+	SPVC_COMPILER_OPTION_MSL_FORCE_ACTIVE_ARGUMENT_BUFFER_RESOURCES = 50 | SPVC_COMPILER_OPTION_MSL_BIT,
 
 	SPVC_COMPILER_OPTION_INT_MAX = 0x7fffffff
 } spvc_compiler_option;

spirv_msl.cpp

@@ -1003,6 +1003,9 @@ string CompilerMSL::compile()
 	fixup_image_load_store_access();
 
 	set_enabled_interface_variables(get_active_interface_variables());
+	if (msl_options.force_active_argument_buffer_resources)
+		activate_argument_buffer_resources();
+
 	if (swizzle_buffer_id)
 		active_interface_variables.insert(swizzle_buffer_id);
 	if (buffer_size_buffer_id)
@@ -12691,3 +12694,17 @@ void CompilerMSL::analyze_argument_buffers()
 		}
 	}
 }
+
+void CompilerMSL::activate_argument_buffer_resources()
+{
+	// For ABI compatibility, force-enable all resources which are part of argument buffers.
+	ir.for_each_typed_id<SPIRVariable>([&](uint32_t self, const SPIRVariable &) {
+		if (!has_decoration(self, DecorationDescriptorSet))
+			return;
+
+		uint32_t desc_set = get_decoration(self, DecorationDescriptorSet);
+		if (descriptor_set_is_argument_buffer(desc_set))
+			active_interface_variables.insert(self);
+	});
+}
+

spirv_msl.hpp

@@ -307,6 +307,11 @@ class CompilerMSL : public CompilerGLSL
 		// Requires MSL 2.1, use the native support for texel buffers.
 		bool texture_buffer_native = false;
 
+		// Forces all resources which are part of an argument buffer to be considered active.
+		// This ensures ABI compatibility between shaders where some resources might be unused,
+		// and would otherwise declare a different IAB.
+		bool force_active_argument_buffer_resources = false;
+
 		bool is_ios()
 		{
 			return platform == iOS;
@@ -863,6 +868,8 @@ class CompilerMSL : public CompilerGLSL
 
 	void add_spv_func_and_recompile(SPVFuncImpl spv_func);
 
+	void activate_argument_buffer_resources();
+
 	// OpcodeHandler that handles several MSL preprocessing operations.
 	struct OpCodePreprocessor : OpcodeHandler
 	{

test_shaders.py

@@ -226,6 +226,8 @@ def cross_compile_msl(shader, spirv, opt, iterations, paths):
         msl_args.append('2')
         msl_args.append('--msl-discrete-descriptor-set')
         msl_args.append('3')
+    if '.force-active.' in shader:
+        msl_args.append('--msl-force-active-argument-buffer-resources')
     if '.line.' in shader:
         msl_args.append('--emit-line-directives')
     if '.multiview.' in shader: