Skip to content

Commit

Permalink
MSL: Fix regression error in argument buffer runtime arrays.
Browse files Browse the repository at this point in the history
Argument buffers can contain multiple runtime arrays if they have fixed
lengths as specified by the binding API. Regression error had assumed each
runtime array is in separate argument buffer with undefined array length.

- Add CompilerMSL::is_var_runtime_size_array() to include test for
  setting of array length via CompilerMSL::add_msl_resource_binding().

- Fixed unrelated test case MSL compile syntax failure when acceleration
  structure is the first entry point function argument (unrelated).
  • Loading branch information
billhollings committed Nov 3, 2023
1 parent 637cff3 commit 4a42191
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ uint proceeFn(thread raytracing::intersection_query<raytracing::instancing, rayt
return _50;
}

fragment void main0(, raytracing::acceleration_structure<raytracing::instancing> topLevelAS [[buffer(0)]])
fragment void main0(raytracing::acceleration_structure<raytracing::instancing> topLevelAS [[buffer(0)]])
{
raytracing::intersection_query<raytracing::instancing, raytracing::triangle_data> rayQuery;
initFn(rayQuery, topLevelAS);
Expand Down
38 changes: 23 additions & 15 deletions spirv_msl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ bool CompilerMSL::is_msl_resource_binding_used(ExecutionModel model, uint32_t de
return itr != end(resource_bindings) && itr->second.second;
}

bool CompilerMSL::is_var_runtime_size_array(const SPIRVariable &var) const
{
return is_runtime_size_array(get_variable_data_type(var)) && get_resource_array_size(var.self) == 0;
}

// Returns the size of the array of resources used by the variable with the specified id.
// The returned value is retrieved from the resource binding added using add_msl_resource_binding().
uint32_t CompilerMSL::get_resource_array_size(uint32_t id) const
Expand Down Expand Up @@ -1361,7 +1366,7 @@ void CompilerMSL::emit_entry_point_declarations()
const auto &type = get_variable_data_type(var);
const auto &buffer_type = get_variable_element_type(var);
const string name = to_name(var.self);
if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
{
if (msl_options.argument_buffers_tier < Options::ArgumentBuffersTier::Tier2)
{
Expand Down Expand Up @@ -10503,7 +10508,7 @@ void CompilerMSL::emit_function_prototype(SPIRFunction &func, const Bitset &)
// Manufacture automatic sampler arg for SampledImage texture
if (arg_type.image.dim != DimBuffer)
{
if (arg_type.array.empty() || is_runtime_size_array(arg_type))
if (arg_type.array.empty() || (var ? is_var_runtime_size_array(*var) : is_runtime_size_array(arg_type)))
{
decl += join(", ", sampler_type(arg_type, arg.id), " ", to_sampler_expression(name_id));
}
Expand Down Expand Up @@ -11693,8 +11698,7 @@ string CompilerMSL::to_buffer_size_expression(uint32_t id)
auto array_expr = expr.substr(index);
if (auto var = maybe_get_backing_variable(id))
{
auto &var_type = get<SPIRType>(var->basetype);
if (is_runtime_size_array(var_type))
if (is_var_runtime_size_array(*var))
{
if (!msl_options.runtime_array_rich_descriptor)
SPIRV_CROSS_THROW("OpArrayLength requires rich descriptor format");
Expand Down Expand Up @@ -13227,7 +13231,7 @@ void CompilerMSL::entry_point_args_discrete_descriptors(string &ep_args)
uint32_t array_size = to_array_size_literal(type);

is_using_builtin_array = true;
if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
{
add_spv_func_and_recompile(SPVFuncImplVariableDescriptorArray);
if (!ep_args.empty())
Expand Down Expand Up @@ -13289,7 +13293,7 @@ void CompilerMSL::entry_point_args_discrete_descriptors(string &ep_args)
if (!ep_args.empty())
ep_args += ", ";
ep_args += sampler_type(type, var_id) + " " + r.name;
if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
ep_args += "_ [[buffer(" + convert_to_string(r.index) + ")]]";
else
ep_args += " [[sampler(" + convert_to_string(r.index) + ")]]";
Expand All @@ -13307,7 +13311,7 @@ void CompilerMSL::entry_point_args_discrete_descriptors(string &ep_args)
if (r.plane > 0)
ep_args += join(plane_name_suffix, r.plane);

if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
ep_args += "_ [[buffer(" + convert_to_string(r.index) + ")";
else
ep_args += " [[texture(" + convert_to_string(r.index) + ")";
Expand Down Expand Up @@ -13338,17 +13342,21 @@ void CompilerMSL::entry_point_args_discrete_descriptors(string &ep_args)
}
case SPIRType::AccelerationStructure:
{
if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
{
add_spv_func_and_recompile(SPVFuncImplVariableDescriptor);
const auto &parent_type = get<SPIRType>(type.parent_type);
ep_args += ", const device spvDescriptor<" + type_to_glsl(parent_type) + ">* " +
if (!ep_args.empty())
ep_args += ", ";
ep_args += "const device spvDescriptor<" + type_to_glsl(parent_type) + ">* " +
to_restrict(var_id, true) + r.name + "_";
ep_args += " [[buffer(" + convert_to_string(r.index) + ")]]";
}
else
{
ep_args += ", " + type_to_glsl(type, var_id) + " " + r.name;
if (!ep_args.empty())
ep_args += ", ";
ep_args += type_to_glsl(type, var_id) + " " + r.name;
ep_args += " [[buffer(" + convert_to_string(r.index) + ")]]";
}
break;
Expand Down Expand Up @@ -13440,7 +13448,7 @@ void CompilerMSL::fix_up_shader_inputs_outputs()
entry_func.fixup_hooks_in.push_back(
[this, &type, &var, var_id]()
{
bool is_array_type = !type.array.empty() && !is_runtime_size_array(type);
bool is_array_type = !type.array.empty() && !is_var_runtime_size_array(var);

uint32_t desc_set = get_decoration(var_id, DecorationDescriptorSet);
if (descriptor_set_is_argument_buffer(desc_set))
Expand Down Expand Up @@ -14056,7 +14064,7 @@ uint32_t CompilerMSL::get_metal_resource_index(SPIRVariable &var, SPIRType::Base
}
else
{
if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
{
basetype = SPIRType::Struct;
binding_stride = 1;
Expand Down Expand Up @@ -14214,7 +14222,7 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg)
else
decl = join(cv_qualifier, type_to_glsl(type, arg.id));
}
else if (is_runtime_size_array(type))
else if (is_var_runtime_size_array(var))
{
const auto *parent_type = &get<SPIRType>(type.parent_type);
auto type_name = type_to_glsl(*parent_type, arg.id);
Expand Down Expand Up @@ -14316,7 +14324,7 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg)
decl += join("[", array_size, "]");
}
}
else if (is_runtime_size_array(type))
else if (is_var_runtime_size_array(var))
{
decl += " " + to_expression(name_id);
}
Expand Down Expand Up @@ -14366,7 +14374,7 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg)
}
else if (type_is_image || type_is_tlas)
{
if (is_runtime_size_array(type))
if (is_var_runtime_size_array(var))
{
decl = address_space + " " + decl + " " + to_expression(name_id);
}
Expand Down
1 change: 1 addition & 0 deletions spirv_msl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,7 @@ class CompilerMSL : public CompilerGLSL
void emit_specialization_constants_and_structs();
void emit_interface_block(uint32_t ib_var_id);
bool maybe_emit_array_assignment(uint32_t id_lhs, uint32_t id_rhs);
bool is_var_runtime_size_array(const SPIRVariable &var) const;
uint32_t get_resource_array_size(uint32_t id) const;

void fix_up_shader_inputs_outputs();
Expand Down

0 comments on commit 4a42191

Please sign in to comment.