From aa271c1460bbf9666f82f8430c3c42a22d694da8 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Wed, 17 Feb 2021 11:29:33 +0100 Subject: [PATCH 1/2] MSL: Refactor out location consumption count computation. --- spirv_msl.cpp | 42 ++++++++++++++++++------------------------ spirv_msl.hpp | 3 ++- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 13de6408..812302af 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -100,7 +100,7 @@ void CompilerMSL::set_argument_buffer_device_address_space(uint32_t desc_set, bo bool CompilerMSL::is_msl_shader_input_used(uint32_t location) { - return inputs_in_use.count(location) != 0; + return location_inputs_in_use.count(location) != 0; } bool CompilerMSL::is_msl_resource_binding_used(ExecutionModel model, uint32_t desc_set, uint32_t binding) const @@ -1801,34 +1801,28 @@ void CompilerMSL::mark_as_packable(SPIRType &type) } } +uint32_t CompilerMSL::type_to_location_count(const SPIRType &type) const +{ + // In MSL, we cannot place structs in any context where we need locations. + assert(type.basetype != SPIRType::Struct); + + uint32_t dim = 1; + for (uint32_t i = 0; i < type.array.size(); i++) + dim *= to_array_size_literal(type, i); + + uint32_t count = dim * type.columns; + return count; +} + // If a shader input exists at the location, it is marked as being used by this shader void CompilerMSL::mark_location_as_used_by_shader(uint32_t location, const SPIRType &type, StorageClass storage) { if (storage != StorageClassInput) return; - if (is_array(type)) - { - uint32_t dim = 1; - for (uint32_t i = 0; i < type.array.size(); i++) - dim *= to_array_size_literal(type, i); - for (uint32_t i = 0; i < dim; i++) - { - if (is_matrix(type)) - { - for (uint32_t j = 0; j < type.columns; j++) - inputs_in_use.insert(location++); - } - else - inputs_in_use.insert(location++); - } - } - else if (is_matrix(type)) - { - for (uint32_t i = 0; i < type.columns; i++) - inputs_in_use.insert(location + i); - } - else - inputs_in_use.insert(location); + + uint32_t count = type_to_location_count(type); + for (uint32_t i = 0; i < count; i++) + location_inputs_in_use.insert(location + i); } uint32_t CompilerMSL::get_target_components_for_fragment_location(uint32_t location) const diff --git a/spirv_msl.hpp b/spirv_msl.hpp index 40ba4552..2b7642be 100644 --- a/spirv_msl.hpp +++ b/spirv_msl.hpp @@ -929,13 +929,14 @@ protected: // Must be ordered to ensure declarations are in a specific order. std::map inputs_by_location; std::unordered_map inputs_by_builtin; - std::unordered_set inputs_in_use; + std::unordered_set location_inputs_in_use; std::unordered_map fragment_output_components; std::set pragma_lines; std::set typedef_lines; SmallVector vars_needing_early_declaration; std::unordered_map, InternalHasher> resource_bindings; + uint32_t type_to_location_count(const SPIRType &type) const; uint32_t next_metal_resource_index_buffer = 0; uint32_t next_metal_resource_index_texture = 0; From ce552f4f918bb92479771bad7f7061ae1a505bc2 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Wed, 17 Feb 2021 12:21:21 +0100 Subject: [PATCH 2/2] MSL: Gracefully assign automatic input locations to builtin attributes. --- ...-input-automatic-attribute-assignment.tese | 31 +++++ ...-input-automatic-attribute-assignment.tese | 10 ++ spirv_msl.cpp | 112 ++++++++++++++---- spirv_msl.hpp | 12 +- 4 files changed, 143 insertions(+), 22 deletions(-) create mode 100644 reference/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese create mode 100644 shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese diff --git a/reference/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese b/reference/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese new file mode 100644 index 00000000..5816a724 --- /dev/null +++ b/reference/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese @@ -0,0 +1,31 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 gl_Position [[position]]; +}; + +struct main0_in +{ + float4 FragColors [[attribute(2)]]; + float4 gl_Position [[attribute(1)]]; +}; + +struct main0_patchIn +{ + float4 FragColor [[attribute(0)]]; + float2 gl_TessLevelInner [[attribute(3)]]; + float4 gl_TessLevelOuter [[attribute(4)]]; + patch_control_point gl_in; +}; + +[[ patch(quad, 0) ]] vertex main0_out main0(main0_patchIn patchIn [[stage_in]], uint gl_PrimitiveID [[patch_id]]) +{ + main0_out out = {}; + out.gl_Position = (((((float4(1.0) + patchIn.FragColor) + patchIn.gl_in[0].FragColors) + patchIn.gl_in[1].FragColors) + float4(patchIn.gl_TessLevelInner.x)) + float4(patchIn.gl_TessLevelOuter[int(gl_PrimitiveID) & 1])) + patchIn.gl_in[0].gl_Position; + return out; +} + diff --git a/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese b/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese new file mode 100644 index 00000000..1d8a5006 --- /dev/null +++ b/shaders-msl-no-opt/tese/builtin-input-automatic-attribute-assignment.tese @@ -0,0 +1,10 @@ +#version 450 +layout(quads) in; + +layout(location = 0) patch in vec4 FragColor; +layout(location = 2) in vec4 FragColors[]; + +void main() +{ + gl_Position = vec4(1.0) + FragColor + FragColors[0] + FragColors[1] + gl_TessLevelInner[0] + gl_TessLevelOuter[gl_PrimitiveID & 1] + gl_in[0].gl_Position; +} diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 812302af..4c660a6e 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -103,6 +103,15 @@ bool CompilerMSL::is_msl_shader_input_used(uint32_t location) return location_inputs_in_use.count(location) != 0; } +uint32_t CompilerMSL::get_automatic_builtin_input_location(spv::BuiltIn builtin) const +{ + auto itr = builtin_to_automatic_input_location.find(builtin); + if (itr == builtin_to_automatic_input_location.end()) + return k_unknown_location; + else + return itr->second; +} + bool CompilerMSL::is_msl_resource_binding_used(ExecutionModel model, uint32_t desc_set, uint32_t binding) const { StageSetBinding tuple = { model, desc_set, binding }; @@ -2720,7 +2729,7 @@ void CompilerMSL::add_tess_level_input_to_interface_block(const std::string &ib_ // Force the variable to have the proper name. set_name(var.self, builtin_to_glsl(builtin, StorageClassFunction)); - if (get_entry_point().flags.get(ExecutionModeTriangles)) + if (get_execution_mode_bitset().get(ExecutionModeTriangles)) { // Triangles are tricky, because we want only one member in the struct. @@ -2743,6 +2752,10 @@ void CompilerMSL::add_tess_level_input_to_interface_block(const std::string &ib_ // Give the member a name set_member_name(ib_type.self, ib_mbr_idx, mbr_name); + // We cannot decorate both, but the important part is that + // it's marked as builtin so we can get automatic attribute assignment if needed. + set_member_decoration(ib_type.self, ib_mbr_idx, DecorationBuiltIn, builtin); + // There is no qualified alias since we need to flatten the internal array on return. if (get_decoration_bitset(var.self).get(DecorationLocation)) { @@ -2805,6 +2818,8 @@ void CompilerMSL::add_tess_level_input_to_interface_block(const std::string &ib_ string qual_var_name = ib_var_ref + "." + mbr_name; ir.meta[var.self].decoration.qualified_alias = qual_var_name; + set_member_decoration(ib_type.self, ib_mbr_idx, DecorationBuiltIn, builtin); + if (get_decoration_bitset(var.self).get(DecorationLocation)) { uint32_t locn = get_decoration(var.self, DecorationLocation); @@ -10041,7 +10056,13 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in return ""; } } - uint32_t locn = get_ordered_member_location(type.self, index); + + uint32_t locn; + if (is_builtin) + locn = get_or_allocate_builtin_input_member_location(builtin, type.self, index); + else + locn = get_member_location(type.self, index); + if (locn != k_unknown_location) return string(" [[attribute(") + convert_to_string(locn) + ")]]"; } @@ -10081,7 +10102,7 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in } } uint32_t comp; - uint32_t locn = get_ordered_member_location(type.self, index, &comp); + uint32_t locn = get_member_location(type.self, index, &comp); if (locn != k_unknown_location) { if (comp != k_unknown_component) @@ -10117,7 +10138,13 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in } if (msl_options.multi_patch_workgroup) return ""; - uint32_t locn = get_ordered_member_location(type.self, index); + + uint32_t locn; + if (is_builtin) + locn = get_or_allocate_builtin_input_member_location(builtin, type.self, index); + else + locn = get_member_location(type.self, index); + if (locn != k_unknown_location) return string(" [[attribute(") + convert_to_string(locn) + ")]]"; } @@ -10150,7 +10177,13 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in // The special control point array must not be marked with an attribute. if (get_type(type.member_types[index]).basetype == SPIRType::ControlPointArray) return ""; - uint32_t locn = get_ordered_member_location(type.self, index); + + uint32_t locn; + if (is_builtin) + locn = get_or_allocate_builtin_input_member_location(builtin, type.self, index); + else + locn = get_member_location(type.self, index); + if (locn != k_unknown_location) return string(" [[attribute(") + convert_to_string(locn) + ")]]"; } @@ -10190,7 +10223,7 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in else { uint32_t comp; - uint32_t locn = get_ordered_member_location(type.self, index, &comp); + uint32_t locn = get_member_location(type.self, index, &comp); if (locn != k_unknown_location) { // For user-defined attributes, this is fine. From Vulkan spec: @@ -10292,7 +10325,7 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in return ""; } } - uint32_t locn = get_ordered_member_location(type.self, index); + uint32_t locn = get_member_location(type.self, index); // Metal will likely complain about missing color attachments, too. if (locn != k_unknown_location && !(msl_options.enable_frag_output_mask & (1 << locn))) return ""; @@ -10341,24 +10374,61 @@ string CompilerMSL::member_attribute_qualifier(const SPIRType &type, uint32_t in // If the location of the member has been explicitly set, that location is used. If not, this // function assumes the members are ordered in their location order, and simply returns the // index as the location. -uint32_t CompilerMSL::get_ordered_member_location(uint32_t type_id, uint32_t index, uint32_t *comp) +uint32_t CompilerMSL::get_member_location(uint32_t type_id, uint32_t index, uint32_t *comp) const { - auto &m = ir.meta[type_id]; - if (index < m.members.size()) + if (comp) { - auto &dec = m.members[index]; - if (comp) - { - if (dec.decoration_flags.get(DecorationComponent)) - *comp = dec.component; - else - *comp = k_unknown_component; - } - if (dec.decoration_flags.get(DecorationLocation)) - return dec.location; + if (has_member_decoration(type_id, index, DecorationComponent)) + *comp = get_member_decoration(type_id, index, DecorationComponent); + else + *comp = k_unknown_component; } - return index; + if (has_member_decoration(type_id, index, DecorationLocation)) + return get_member_decoration(type_id, index, DecorationLocation); + else + return k_unknown_location; +} + +uint32_t CompilerMSL::get_or_allocate_builtin_input_member_location(spv::BuiltIn builtin, + uint32_t type_id, uint32_t index, + uint32_t *comp) +{ + uint32_t loc = get_member_location(type_id, index, comp); + if (loc != k_unknown_location) + return loc; + + if (comp) + *comp = k_unknown_component; + + // Late allocation. Find a location which is unused by the application. + // This can happen for built-in inputs in tessellation which are mixed and matched with user inputs. + auto &mbr_type = get(get(type_id).member_types[index]); + uint32_t count = type_to_location_count(mbr_type); + + // This should always be 1. + if (count != 1) + return k_unknown_location; + + loc = 0; + while (location_inputs_in_use.count(loc) != 0) + loc++; + + set_member_decoration(type_id, index, DecorationLocation, loc); + + // Triangle tess level inputs are shared in one packed float4, + // mark both builtins as sharing one location. + if (get_execution_mode_bitset().get(ExecutionModeTriangles) && + (builtin == BuiltInTessLevelInner || builtin == BuiltInTessLevelOuter)) + { + builtin_to_automatic_input_location[BuiltInTessLevelInner] = loc; + builtin_to_automatic_input_location[BuiltInTessLevelOuter] = loc; + } + else + builtin_to_automatic_input_location[builtin] = loc; + + mark_location_as_used_by_shader(loc, mbr_type, StorageClassInput); + return loc; } // Returns the type declaration for a function, including the diff --git a/spirv_msl.hpp b/spirv_msl.hpp index 2b7642be..23777b89 100644 --- a/spirv_msl.hpp +++ b/spirv_msl.hpp @@ -550,6 +550,13 @@ public: // Query after compilation is done. This allows you to check if an input location was used by the shader. bool is_msl_shader_input_used(uint32_t location); + // If not using add_msl_shader_input, it's possible + // that certain builtin attributes need to be automatically assigned locations. + // This is typical for tessellation builtin inputs such as tess levels, gl_Position, etc. + // This returns k_unknown_location if the location was explicitly assigned with + // add_msl_shader_input or the builtin is not used, otherwise returns N in [[attribute(N)]]. + uint32_t get_automatic_builtin_input_location(spv::BuiltIn builtin) const; + // NOTE: Only resources which are remapped using add_msl_resource_binding will be reported here. // Constexpr samplers are always assumed to be emitted. // No specific MSLResourceBinding remapping is required for constexpr samplers as long as they are remapped @@ -826,7 +833,9 @@ protected: std::string argument_decl(const SPIRFunction::Parameter &arg); std::string round_fp_tex_coords(std::string tex_coords, bool coord_is_fp); uint32_t get_metal_resource_index(SPIRVariable &var, SPIRType::BaseType basetype, uint32_t plane = 0); - uint32_t get_ordered_member_location(uint32_t type_id, uint32_t index, uint32_t *comp = nullptr); + uint32_t get_member_location(uint32_t type_id, uint32_t index, uint32_t *comp = nullptr) const; + uint32_t get_or_allocate_builtin_input_member_location(spv::BuiltIn builtin, + uint32_t type_id, uint32_t index, uint32_t *comp = nullptr); // MSL packing rules. These compute the effective packing rules as observed by the MSL compiler in the MSL output. // These values can change depending on various extended decorations which control packing rules. @@ -931,6 +940,7 @@ protected: std::unordered_map inputs_by_builtin; std::unordered_set location_inputs_in_use; std::unordered_map fragment_output_components; + std::unordered_map builtin_to_automatic_input_location; std::set pragma_lines; std::set typedef_lines; SmallVector vars_needing_early_declaration;