MSL: Metal argument buffer use enhancements #1596
Conversation
Add MSLTextureType enum and get_msl_texture_type() function.
…arguments with their assigned index values, id(N). Add CompilerMSL::Options::pad_argument_buffer_resources to enable such padding.
…lings/SPIRV-Cross into msl-arg-buff-updates
…IRV-Cross into msl-arg-buff-updates
…into msl-arg-buff-updates
|
First comment right off the bat. Lots of merge commits in the PR. Please rebase the PR. |
spirv_msl.hpp
Outdated
| @@ -556,6 +580,9 @@ class CompilerMSL : public CompilerGLSL | |||
| // by remap_constexpr_sampler(_by_binding). | |||
| bool is_msl_resource_binding_used(spv::ExecutionModel model, uint32_t set, uint32_t binding) const; | |||
|
|
|||
| // Returns the MSL texture type of the variable at the desc_set and binding | |||
| MSLTextureType get_msl_texture_type(uint32_t desc_set, uint32_t binding); | |||
There was a problem hiding this comment.
Should probably be const?
spirv_msl.cpp
Outdated
| has_decoration(self, DecorationBinding) && (get_decoration(self, DecorationBinding) == binding)) | ||
| msl_tex_type = get_msl_texture_type(get_variable_data_type(var)); | ||
| }); | ||
| return msl_tex_type; |
spirv_msl.cpp
Outdated
| ir.for_each_typed_id<SPIRVariable>([&](uint32_t self, SPIRVariable &var) { | ||
| if (has_decoration(self, DecorationDescriptorSet) && (get_decoration(self, DecorationDescriptorSet) == desc_set) && | ||
| has_decoration(self, DecorationBinding) && (get_decoration(self, DecorationBinding) == binding)) | ||
| msl_tex_type = get_msl_texture_type(get_variable_data_type(var)); |
There was a problem hiding this comment.
Braces for clarity.
spirv_msl.hpp
Outdated
| @@ -95,6 +95,21 @@ enum MSLSamplerCoord | |||
| MSL_SAMPLER_INT_MAX = 0x7fffffff | |||
| }; | |||
|
|
|||
| enum MSLTextureType | |||
There was a problem hiding this comment.
Why is this call useful? It seems like it's a convenience query for apps?
| bool use_array = pad_cnt > 1; | ||
| uint32_t pad_type_id = ir.increase_bound_by(use_array ? 2 : 1); | ||
| auto &pad_type = set<SPIRType>(pad_type_id); | ||
| pad_type.basetype = SPIRType::UInt64; |
There was a problem hiding this comment.
This is questionable. Doesn't this basically assume that descriptors take up 8 bytes? I would normally expect this kind of interface to specify the descriptor type used for specific IDs.
There was a problem hiding this comment.
AMD texture descriptors are 16 or 32 bytes for example.
|
I pulled the texture type and handled it outside using reflection. Turns out there is also still work to be done on the question of padding. So for now, I'm going to close this PR and resubmit a variation if it turns out to be needed. |
Add query for type of MSL texture on desc set bindings:
Support padding between argument buffer struct members to align arguments: