From 548a23da341d9215efc89bca16cab3d0cf446db4 Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Mon, 20 Sep 2021 17:57:11 -0400 Subject: [PATCH] MSL: Track location component to match vecsize between shader stages. Matching output/input struct member types between shader stages could fail if a location is shared between members, each using different components of that location, because the member vecsize was only stored once for the location. Add MSLShaderInput::component member. Use LocationComponentPair to key inputs_by_location, instead of just location. ensure_correct_input_type() pass component value as well as location. --- spirv_msl.cpp | 37 ++++++++++++++++++++++--------------- spirv_msl.hpp | 5 +++-- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 9e797f66..e37b13ed 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -58,7 +58,7 @@ CompilerMSL::CompilerMSL(ParsedIR &&ir_) void CompilerMSL::add_msl_shader_input(const MSLShaderInput &si) { - inputs_by_location[si.location] = si; + inputs_by_location[{si.location, si.component}] = si; if (si.builtin != BuiltInMax && !inputs_by_builtin.count(si.builtin)) inputs_by_builtin[si.builtin] = si; } @@ -2224,9 +2224,10 @@ void CompilerMSL::add_plain_variable_to_interface_block(StorageClass storage, co if (get_decoration_bitset(var.self).get(DecorationLocation)) { uint32_t locn = get_decoration(var.self, DecorationLocation); + uint32_t comp = get_decoration(var.self, DecorationComponent); if (storage == StorageClassInput) { - type_id = ensure_correct_input_type(var.basetype, locn, 0, meta.strip_array); + type_id = ensure_correct_input_type(var.basetype, locn, comp, 0, meta.strip_array); var.basetype = type_id; type_id = get_pointee_type_id(type_id); @@ -2238,6 +2239,8 @@ void CompilerMSL::add_plain_variable_to_interface_block(StorageClass storage, co ib_type.member_types[ib_mbr_idx] = type_id; } set_member_decoration(ib_type.self, ib_mbr_idx, DecorationLocation, locn); + if (comp) + set_member_decoration(ib_type.self, ib_mbr_idx, DecorationComponent, comp); mark_location_as_used_by_shader(locn, get(type_id), storage); } else if (is_builtin && is_tessellation_shader() && inputs_by_builtin.count(builtin)) @@ -2393,8 +2396,8 @@ void CompilerMSL::add_composite_variable_to_interface_block(StorageClass storage uint32_t comp = get_decoration(var.self, DecorationComponent); if (storage == StorageClassInput) { - var.basetype = ensure_correct_input_type(var.basetype, locn, 0, meta.strip_array); - uint32_t mbr_type_id = ensure_correct_input_type(usable_type->self, locn, 0, meta.strip_array); + var.basetype = ensure_correct_input_type(var.basetype, locn, comp, 0, meta.strip_array); + uint32_t mbr_type_id = ensure_correct_input_type(usable_type->self, locn, comp, 0, meta.strip_array); if (storage == StorageClassInput && pull_model_inputs.count(var.self)) ib_type.member_types[ib_mbr_idx] = build_msl_interpolant_type(mbr_type_id, is_noperspective); else @@ -2739,9 +2742,10 @@ void CompilerMSL::add_plain_member_variable_to_interface_block(StorageClass stor if (has_member_decoration(var_type.self, mbr_idx, DecorationLocation)) { uint32_t locn = get_member_decoration(var_type.self, mbr_idx, DecorationLocation); + uint32_t comp = get_member_decoration(var_type.self, mbr_idx, DecorationComponent); if (storage == StorageClassInput) { - mbr_type_id = ensure_correct_input_type(mbr_type_id, locn, 0, meta.strip_array); + mbr_type_id = ensure_correct_input_type(mbr_type_id, locn, comp, 0, meta.strip_array); var_type.member_types[mbr_idx] = mbr_type_id; if (storage == StorageClassInput && pull_model_inputs.count(var.self)) ib_type.member_types[ib_mbr_idx] = build_msl_interpolant_type(mbr_type_id, is_noperspective); @@ -2758,7 +2762,7 @@ void CompilerMSL::add_plain_member_variable_to_interface_block(StorageClass stor uint32_t locn = get_accumulated_member_location(var, mbr_idx, meta.strip_array); if (storage == StorageClassInput) { - mbr_type_id = ensure_correct_input_type(mbr_type_id, locn, 0, meta.strip_array); + mbr_type_id = ensure_correct_input_type(mbr_type_id, locn, 0, 0, meta.strip_array); var_type.member_types[mbr_idx] = mbr_type_id; if (storage == StorageClassInput && pull_model_inputs.count(var.self)) ib_type.member_types[ib_mbr_idx] = build_msl_interpolant_type(mbr_type_id, is_noperspective); @@ -3602,7 +3606,7 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage, bool patch) // the struct containing them is the correct size and layout. for (auto &input : inputs_by_location) { - if (location_inputs_in_use.count(input.first) != 0) + if (location_inputs_in_use.count(input.first.location) != 0) continue; // Create a fake variable to put at the location. @@ -3642,7 +3646,10 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage, bool patch) ptr_type.self = array_type_id; auto &fake_var = set(var_id, ptr_type_id, storage); - set_decoration(var_id, DecorationLocation, input.first); + set_decoration(var_id, DecorationLocation, input.first.location); + if (input.first.component) + set_decoration(var_id, DecorationComponent, input.first.component); + meta.strip_array = true; meta.allow_local_declaration = false; add_variable_to_interface_block(storage, ib_var_ref, ib_type, fake_var, meta); @@ -3794,7 +3801,7 @@ uint32_t CompilerMSL::ensure_correct_builtin_type(uint32_t type_id, BuiltIn buil // Ensure that the type is compatible with the shader input. // If it is, simply return the given type ID. // Otherwise, create a new type, and return its ID. -uint32_t CompilerMSL::ensure_correct_input_type(uint32_t type_id, uint32_t location, uint32_t num_components, bool strip_array) +uint32_t CompilerMSL::ensure_correct_input_type(uint32_t type_id, uint32_t location, uint32_t component, uint32_t num_components, bool strip_array) { auto &type = get(type_id); @@ -3804,7 +3811,7 @@ uint32_t CompilerMSL::ensure_correct_input_type(uint32_t type_id, uint32_t locat if (type.basetype == SPIRType::Struct || type.array.size() > max_array_dimensions) return type_id; - auto p_va = inputs_by_location.find(location); + auto p_va = inputs_by_location.find({location, component}); if (p_va == end(inputs_by_location)) { if (num_components > type.vecsize) @@ -14537,11 +14544,11 @@ SPIRType CompilerMSL::get_presumed_input_type(const SPIRType &ib_type, uint32_t { SPIRType type = get_physical_member_type(ib_type, index); uint32_t loc = get_member_decoration(ib_type.self, index, DecorationLocation); - if (inputs_by_location.count(loc)) - { - if (inputs_by_location.at(loc).vecsize > type.vecsize) - type.vecsize = inputs_by_location.at(loc).vecsize; - } + uint32_t cmp = get_member_decoration(ib_type.self, index, DecorationComponent); + auto p_va = inputs_by_location.find({loc, cmp}); + if (p_va != end(inputs_by_location) && p_va->second.vecsize > type.vecsize) + type.vecsize = p_va->second.vecsize; + return type; } diff --git a/spirv_msl.hpp b/spirv_msl.hpp index 9dc1a1a0..a2b1b551 100644 --- a/spirv_msl.hpp +++ b/spirv_msl.hpp @@ -60,6 +60,7 @@ enum MSLShaderInputFormat struct MSLShaderInput { uint32_t location = 0; + uint32_t component = 0; MSLShaderInputFormat format = MSL_SHADER_INPUT_FORMAT_OTHER; spv::BuiltIn builtin = spv::BuiltInMax; uint32_t vecsize = 0; @@ -837,7 +838,7 @@ protected: void mark_location_as_used_by_shader(uint32_t location, const SPIRType &type, spv::StorageClass storage, bool fallback = false); uint32_t ensure_correct_builtin_type(uint32_t type_id, spv::BuiltIn builtin); - uint32_t ensure_correct_input_type(uint32_t type_id, uint32_t location, + uint32_t ensure_correct_input_type(uint32_t type_id, uint32_t location, uint32_t component, uint32_t num_components, bool strip_array); void emit_custom_templates(); @@ -980,7 +981,7 @@ protected: Options msl_options; std::set spv_function_implementations; // Must be ordered to ensure declarations are in a specific order. - std::map inputs_by_location; + std::map inputs_by_location; std::unordered_map inputs_by_builtin; std::unordered_set location_inputs_in_use; std::unordered_set location_inputs_in_use_fallback;