Merge pull request #1040 from KhronosGroup/fix-1037

MSL: Support custom bindings for argument buffer itself.

CMakeLists.txt

@@ -287,7 +287,7 @@ if (SPIRV_CROSS_STATIC)
 endif()
 
 set(spirv-cross-abi-major 0)
-set(spirv-cross-abi-minor 14)
+set(spirv-cross-abi-minor 15)
 set(spirv-cross-abi-patch 0)
 
 if (SPIRV_CROSS_SHARED)
@@ -457,6 +457,10 @@ if (SPIRV_CROSS_CLI)
 				target_link_libraries(spirv-cross-msl-constexpr-test spirv-cross-c)
 				set_target_properties(spirv-cross-msl-constexpr-test PROPERTIES LINK_FLAGS "${spirv-cross-link-flags}")
 
+				add_executable(spirv-cross-msl-resource-binding-test tests-other/msl_resource_bindings.cpp)
+				target_link_libraries(spirv-cross-msl-resource-binding-test spirv-cross-c)
+				set_target_properties(spirv-cross-msl-resource-binding-test PROPERTIES LINK_FLAGS "${spirv-cross-link-flags}")
+
 				if (CMAKE_COMPILER_IS_GNUCXX OR (${CMAKE_CXX_COMPILER_ID} MATCHES "Clang"))
 					target_compile_options(spirv-cross-c-api-test PRIVATE -std=c89 -Wall -Wextra)
 				endif()
@@ -469,6 +473,8 @@ if (SPIRV_CROSS_CLI)
 						COMMAND $<TARGET_FILE:spirv-cross-small-vector-test>)
 				add_test(NAME spirv-cross-msl-constexpr-test
 						COMMAND $<TARGET_FILE:spirv-cross-msl-constexpr-test> ${CMAKE_CURRENT_SOURCE_DIR}/tests-other/msl_constexpr_test.spv)
+				add_test(NAME spirv-cross-msl-resource-binding-test
+						COMMAND $<TARGET_FILE:spirv-cross-msl-resource-binding-test> ${CMAKE_CURRENT_SOURCE_DIR}/tests-other/msl_resource_binding.spv)
 				add_test(NAME spirv-cross-test
 						COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_shaders.py --parallel
 						${spirv-cross-externals}

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 14
+#define SPVC_C_API_VERSION_MINOR 15
 /* Bumped if internal implementation details change. */
 #define SPVC_C_API_VERSION_PATCH 0
 
@@ -303,6 +303,7 @@ SPVC_PUBLIC_API void spvc_msl_resource_binding_init(spvc_msl_resource_binding *b
 #define SPVC_MSL_PUSH_CONSTANT_BINDING (0)
 #define SPVC_MSL_SWIZZLE_BUFFER_BINDING (~(1u))
 #define SPVC_MSL_BUFFER_SIZE_BUFFER_BINDING (~(2u))
+#define SPVC_MSL_ARGUMENT_BUFFER_BINDING (~(3u))
 
 /* Obsolete. Sticks around for backwards compatibility. */
 #define SPVC_MSL_AUX_BUFFER_STRUCT_VERSION 1

spirv_msl.cpp

@@ -6090,6 +6090,7 @@ void CompilerMSL::entry_point_args_builtin(string &ep_args)
 string CompilerMSL::entry_point_args_argument_buffer(bool append_comma)
 {
 	string ep_args = entry_point_arg_stage_in();
+	Bitset claimed_bindings;
 
 	for (uint32_t i = 0; i < kMaxArgumentBuffers; i++)
 	{
@@ -6104,12 +6105,30 @@ string CompilerMSL::entry_point_args_argument_buffer(bool append_comma)
 		if (!ep_args.empty())
 			ep_args += ", ";
 
-		ep_args += get_argument_address_space(var) + " " + type_to_glsl(type) + "& " + to_name(id);
-		ep_args += " [[buffer(" + convert_to_string(i) + ")]]";
+		// Check if the argument buffer binding itself has been remapped.
+		uint32_t buffer_binding;
+		auto itr = resource_bindings.find({ get_entry_point().model, i, kArgumentBufferBinding });
+		if (itr != end(resource_bindings))
+		{
+			buffer_binding = itr->second.first.msl_buffer;
+			itr->second.second = true;
+		}
+		else
+		{
+			// As a fallback, directly map desc set <-> binding.
+			// If that was taken, take the next buffer binding.
+			if (claimed_bindings.get(i))
+				buffer_binding = next_metal_resource_index_buffer;
+			else
+				buffer_binding = i;
+		}
 
-		// Makes it more practical for testing, since the push constant block can occupy the first available
-		// buffer slot if it's not bound explicitly.
-		next_metal_resource_index_buffer = i + 1;
+		claimed_bindings.set(buffer_binding);
+
+		ep_args += get_argument_address_space(var) + " " + type_to_glsl(type) + "& " + to_name(id);
+		ep_args += " [[buffer(" + convert_to_string(buffer_binding) + ")]]";
+
+		next_metal_resource_index_buffer = max(next_metal_resource_index_buffer, buffer_binding + 1);
 	}
 
 	entry_point_args_discrete_descriptors(ep_args);

spirv_msl.hpp

@@ -161,6 +161,12 @@ static const uint32_t kSwizzleBufferBinding = ~(1u);
 // element to indicate the buffer binding for buffer size buffers to support OpArrayLength.
 static const uint32_t kBufferSizeBufferBinding = ~(2u);
 
+// Special constant used in a MSLResourceBinding binding
+// element to indicate the buffer binding used for the argument buffer itself.
+// This buffer binding should be kept as small as possible as all automatic bindings for buffers
+// will start at max(kArgumentBufferBinding) + 1.
+static const uint32_t kArgumentBufferBinding = ~(3u);
+
 static const uint32_t kMaxArgumentBuffers = 8;
 
 // Decompiles SPIR-V to Metal Shading Language

tests-other/msl_resource_bindings.cpp

@@ -0,0 +1,86 @@
+// Testbench for MSL resource binding APIs.
+// It does not validate output at the moment, but it's useful for ad-hoc testing.
+
+#include <spirv_cross_c.h>
+#include <vector>
+#include <stdio.h>
+#include <stdlib.h>
+
+#define SPVC_CHECKED_CALL(x) do { \
+	if ((x) != SPVC_SUCCESS) { \
+		fprintf(stderr, "Failed at line %d.\n", __LINE__); \
+		exit(1); \
+	} \
+} while(0)
+
+static std::vector<SpvId> read_file(const char *path)
+{
+	long len;
+	FILE *file = fopen(path, "rb");
+
+	if (!file)
+		return {};
+
+	fseek(file, 0, SEEK_END);
+	len = ftell(file);
+	rewind(file);
+
+	std::vector<SpvId> buffer(len / sizeof(SpvId));
+	if (fread(buffer.data(), 1, len, file) != (size_t)len)
+	{
+		fclose(file);
+		return {};
+	}
+
+	fclose(file);
+	return buffer;
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2)
+		return EXIT_FAILURE;
+
+	auto buffer = read_file(argv[1]);
+	if (buffer.empty())
+		return EXIT_FAILURE;
+
+	spvc_context ctx;
+	spvc_parsed_ir parsed_ir;
+	spvc_compiler compiler;
+
+	SPVC_CHECKED_CALL(spvc_context_create(&ctx));
+	SPVC_CHECKED_CALL(spvc_context_parse_spirv(ctx, buffer.data(), buffer.size(), &parsed_ir));
+	SPVC_CHECKED_CALL(spvc_context_create_compiler(ctx, SPVC_BACKEND_MSL, parsed_ir, SPVC_CAPTURE_MODE_TAKE_OWNERSHIP, &compiler));
+	SPVC_CHECKED_CALL(spvc_compiler_msl_add_discrete_descriptor_set(compiler, 3));
+
+	spvc_compiler_options opts;
+	SPVC_CHECKED_CALL(spvc_compiler_create_compiler_options(compiler, &opts));
+	SPVC_CHECKED_CALL(spvc_compiler_options_set_bool(opts, SPVC_COMPILER_OPTION_MSL_ARGUMENT_BUFFERS, SPVC_TRUE));
+	SPVC_CHECKED_CALL(spvc_compiler_options_set_uint(opts, SPVC_COMPILER_OPTION_MSL_VERSION, 20000));
+	SPVC_CHECKED_CALL(spvc_compiler_install_compiler_options(compiler, opts));
+
+	spvc_msl_resource_binding binding;
+	spvc_msl_resource_binding_init(&binding);
+	binding.binding = SPVC_MSL_ARGUMENT_BUFFER_BINDING;
+	binding.stage = SpvExecutionModelFragment;
+	binding.desc_set = 0;
+	binding.msl_buffer = 2;
+	SPVC_CHECKED_CALL(spvc_compiler_msl_add_resource_binding(compiler, &binding));
+
+	binding.desc_set = 1;
+	binding.msl_buffer = 3;
+	SPVC_CHECKED_CALL(spvc_compiler_msl_add_resource_binding(compiler, &binding));
+
+	const char *str;
+	SPVC_CHECKED_CALL(spvc_compiler_compile(compiler, &str));
+
+	fprintf(stderr, "Output:\n%s\n", str);
+
+	if (!spvc_compiler_msl_is_resource_used(compiler, SpvExecutionModelFragment, 0, SPVC_MSL_ARGUMENT_BUFFER_BINDING))
+		return EXIT_FAILURE;
+
+	if (!spvc_compiler_msl_is_resource_used(compiler, SpvExecutionModelFragment, 1, SPVC_MSL_ARGUMENT_BUFFER_BINDING))
+		return EXIT_FAILURE;
+}
+