diff --git a/reference/shaders-msl-no-opt/asm/vert/pointer-to-pointer.asm.vert b/reference/shaders-msl-no-opt/asm/vert/pointer-to-pointer.asm.vert new file mode 100644 index 00000000..750afcf2 --- /dev/null +++ b/reference/shaders-msl-no-opt/asm/vert/pointer-to-pointer.asm.vert @@ -0,0 +1,20 @@ +#pragma clang diagnostic ignored "-Wmissing-prototypes" + +#include +#include + +using namespace metal; + +device float* thread * constant _9 = {}; + +static inline __attribute__((always_inline)) +void _10(device float* thread * const thread & _11) +{ +} + +vertex void main0() +{ + device float* thread * _14 = _9; + _10(_14); +} + diff --git a/shaders-msl-no-opt/asm/vert/pointer-to-pointer.asm.vert b/shaders-msl-no-opt/asm/vert/pointer-to-pointer.asm.vert new file mode 100644 index 00000000..22058d2c --- /dev/null +++ b/shaders-msl-no-opt/asm/vert/pointer-to-pointer.asm.vert @@ -0,0 +1,34 @@ +OpCapability Shader +OpCapability VariablePointers +OpCapability VariablePointersStorageBuffer +OpMemoryModel Logical GLSL450 + +OpEntryPoint Vertex %fn_vert "main" + +%F = OpTypeFloat 32 +%PF = OpTypePointer StorageBuffer %F +%PPF = OpTypePointer Private %PF +%PPPF = OpTypePointer Function %PPF + +%V = OpTypeVoid +%Fn0V = OpTypeFunction %V + +%FnArg = OpTypeFunction %V %PPPF + +%uPPF = OpUndef %PPF + +%fn_ptr = OpFunction %V None %FnArg + %arg = OpFunctionParameter %PPPF + %fn_ptr_bb0 = OpLabel + OpReturn +OpFunctionEnd + +%fn_vert = OpFunction %V None %Fn0V + %fn_vert_bb0 = OpLabel + %VPPPF = OpVariable %PPPF Function + OpStore %VPPPF %uPPF + %VV = OpFunctionCall %V %fn_ptr %VPPPF + OpReturn +OpFunctionEnd + + diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index ca8267ec..1b857074 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -12791,7 +12791,7 @@ string CompilerGLSL::variable_decl(const SPIRVariable &variable) // Ignore the pointer type since GLSL doesn't have pointers. auto &type = get_variable_data_type(variable); - if (type.pointer_depth > 1) + if (type.pointer_depth > 1 && !backend.support_pointer_to_pointer) SPIRV_CROSS_THROW("Cannot declare pointer-to-pointer types."); auto res = join(to_qualifiers_glsl(variable.self), variable_decl(type, to_name(variable.self), variable.self)); diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index 00b08197..37ec6a7a 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -568,6 +568,7 @@ protected: bool support_case_fallthrough = true; bool use_array_constructor = false; bool needs_row_major_load_workaround = false; + bool support_pointer_to_pointer = false; } backend; void emit_struct(SPIRType &type); diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 079147fa..1d19e6df 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -399,6 +399,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType vec4_type_ptr; vec4_type_ptr = vec4_type; vec4_type_ptr.pointer = true; + vec4_type_ptr.pointer_depth++; vec4_type_ptr.parent_type = type_id; vec4_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, vec4_type_ptr); @@ -420,6 +421,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -441,6 +443,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -463,6 +466,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -482,6 +486,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -541,6 +546,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr_out; uint_type_ptr_out = get_uint_type(); uint_type_ptr_out.pointer = true; + uint_type_ptr_out.pointer_depth++; uint_type_ptr_out.parent_type = get_uint_type_id(); uint_type_ptr_out.storage = StorageClassOutput; auto &ptr_out_type = set(type_ptr_out_id, uint_type_ptr_out); @@ -572,6 +578,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -631,6 +638,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -652,6 +660,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; auto &ptr_type = set(type_ptr_id, uint_type_ptr); @@ -710,6 +719,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr_out; uint_type_ptr_out = get_uint_type(); uint_type_ptr_out.pointer = true; + uint_type_ptr_out.pointer_depth++; uint_type_ptr_out.parent_type = get_uint_type_id(); uint_type_ptr_out.storage = StorageClassOutput; @@ -731,6 +741,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType uint_type_ptr; uint_type_ptr = get_uint_type(); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = get_uint_type_id(); uint_type_ptr.storage = StorageClassInput; @@ -752,6 +763,7 @@ void CompilerMSL::build_implicit_builtins() uint32_t type_id = build_extended_vector_type(get_uint_type_id(), 3); SPIRType uint_type_ptr = get(type_id); uint_type_ptr.pointer = true; + uint_type_ptr.pointer_depth++; uint_type_ptr.parent_type = type_id; uint_type_ptr.storage = StorageClassInput; @@ -844,6 +856,7 @@ void CompilerMSL::build_implicit_builtins() SPIRType vec4_type_ptr; vec4_type_ptr = vec4_type; vec4_type_ptr.pointer = true; + vec4_type_ptr.pointer_depth++; vec4_type_ptr.parent_type = type_id; vec4_type_ptr.storage = StorageClassOutput; auto &ptr_type = set(type_ptr_id, vec4_type_ptr); @@ -919,7 +932,7 @@ uint32_t CompilerMSL::build_constant_uint_array_pointer() // Create a buffer to hold extra data, including the swizzle constants. SPIRType uint_type_pointer = get_uint_type(); uint_type_pointer.pointer = true; - uint_type_pointer.pointer_depth = 1; + uint_type_pointer.pointer_depth++; uint_type_pointer.parent_type = get_uint_type_id(); uint_type_pointer.storage = StorageClassUniform; set(type_ptr_id, uint_type_pointer); @@ -1278,6 +1291,7 @@ string CompilerMSL::compile() backend.array_is_value_type = !msl_options.force_native_arrays; // Arrays which are part of buffer objects are never considered to be native arrays. backend.buffer_offset_array_is_value_type = false; + backend.support_pointer_to_pointer = true; capture_output_to_buffer = msl_options.capture_output_to_buffer; is_rasterization_disabled = msl_options.disable_rasterization || capture_output_to_buffer; @@ -1739,6 +1753,7 @@ void CompilerMSL::extract_global_variables_from_function(uint32_t func_id, std:: ptr.self = mbr_type_id; ptr.storage = var.storage; ptr.pointer = true; + ptr.pointer_depth++; ptr.parent_type = mbr_type_id; func.add_parameter(mbr_type_id, var_id, true); @@ -1875,6 +1890,7 @@ uint32_t CompilerMSL::build_extended_vector_type(uint32_t type_id, uint32_t comp type->parent_type = new_type_id; type->storage = old_type.storage; type->pointer = true; + type->pointer_depth++; new_type_id = ptr_type_id; } @@ -2803,6 +2819,7 @@ void CompilerMSL::add_tess_level_input_to_interface_block(const std::string &ib_ uint32_t ptr_type_id = ir.increase_bound_by(1); auto &new_var_type = set(ptr_type_id, get(type_id)); new_var_type.pointer = true; + new_var_type.pointer_depth++; new_var_type.storage = StorageClassInput; new_var_type.parent_type = type_id; var.basetype = ptr_type_id; @@ -3310,6 +3327,7 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage, bool patch) set(array_type_id, type); type.pointer = true; + type.pointer_depth++; type.parent_type = array_type_id; type.storage = storage; auto &ptr_type = set(ptr_type_id, type); @@ -3365,6 +3383,7 @@ uint32_t CompilerMSL::add_interface_block_pointer(uint32_t ib_var_id, StorageCla auto &ib_ptr_type = set(ib_ptr_type_id, ib_type); ib_ptr_type.parent_type = ib_ptr_type.type_alias = ib_type.self; ib_ptr_type.pointer = true; + ib_ptr_type.pointer_depth++; ib_ptr_type.storage = storage == StorageClassInput ? (msl_options.multi_patch_workgroup ? StorageClassStorageBuffer : StorageClassWorkgroup) : @@ -3430,6 +3449,7 @@ uint32_t CompilerMSL::ensure_correct_builtin_type(uint32_t type_id, BuiltIn buil auto &ptr_type = set(ptr_type_id); ptr_type = base_type; ptr_type.pointer = true; + ptr_type.pointer_depth++; ptr_type.storage = type.storage; ptr_type.parent_type = base_type_id; return ptr_type_id; @@ -6097,6 +6117,30 @@ void CompilerMSL::emit_custom_functions() } } +static string inject_top_level_storage_qualifier(const string &expr, const string &qualifier) +{ + // Easier to do this through text munging since the qualifier does not exist in the type system at all, + // and plumbing in all that information is not very helpful. + size_t last_reference = expr.find_last_of('&'); + size_t last_pointer = expr.find_last_of('*'); + size_t last_significant = string::npos; + + if (last_reference == string::npos) + last_significant = last_pointer; + else if (last_pointer == string::npos) + last_significant = last_reference; + else + last_significant = std::max(last_reference, last_pointer); + + if (last_significant == string::npos) + return join(qualifier, " ", expr); + else + { + return join(expr.substr(0, last_significant + 1), " ", + qualifier, expr.substr(last_significant + 1, string::npos)); + } +} + // Undefined global memory is not allowed in MSL. // Declare constant and init to zeros. Use {}, as global constructors can break Metal. void CompilerMSL::declare_undefined_values() @@ -6108,7 +6152,10 @@ void CompilerMSL::declare_undefined_values() if (type.basetype == SPIRType::Void) return; - statement("constant ", variable_decl(type, to_name(undef.self), undef.self), " = {};"); + statement(inject_top_level_storage_qualifier( + variable_decl(type, to_name(undef.self), undef.self), + "constant"), + " = {};"); emitted = true; }); @@ -6136,7 +6183,8 @@ void CompilerMSL::declare_constant_arrays() if (!type.array.empty() && (!fully_inlined || is_scalar(type) || is_vector(type))) { auto name = to_name(c.self); - statement("constant ", variable_decl(type, name), " = ", constant_expression(c), ";"); + statement(inject_top_level_storage_qualifier(variable_decl(type, name), "constant"), + " = ", constant_expression(c), ";"); emitted = true; } }); @@ -10600,8 +10648,10 @@ string CompilerMSL::get_type_address_space(const SPIRType &type, uint32_t id, bo } if (!addr_space) + { // No address space for plain values. addr_space = type.pointer || (argument && type.basetype == SPIRType::ControlPointArray) ? "thread" : ""; + } return join(flags.get(DecorationVolatile) || flags.get(DecorationCoherent) ? "volatile " : "", addr_space); } @@ -11930,6 +11980,24 @@ bool CompilerMSL::type_is_msl_framebuffer_fetch(const SPIRType &type) const msl_options.use_framebuffer_fetch_subpasses; } +bool CompilerMSL::type_is_pointer(const SPIRType &type) const +{ + if (!type.pointer) + return false; + auto &parent_type = get(type.parent_type); + // Safeguards when we forget to set pointer_depth (there is an assert for it in type_to_glsl), + // but the extra check shouldn't hurt. + return (type.pointer_depth > parent_type.pointer_depth) || !parent_type.pointer; +} + +bool CompilerMSL::type_is_pointer_to_pointer(const SPIRType &type) const +{ + if (!type.pointer) + return false; + auto &parent_type = get(type.parent_type); + return type.pointer_depth > parent_type.pointer_depth && type_is_pointer(parent_type); +} + string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) { auto &var = get(arg.id); @@ -11956,9 +12024,8 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) if (!type.array.empty() && type_is_image) constref = true; + const char *cv_qualifier = constref ? "const " : ""; string decl; - if (constref) - decl += "const "; // If this is a combined image-sampler for a 2D image with floating-point type, // we emitted the 'spvDynamicImageSampler' type, and this is *not* an alias parameter @@ -11978,22 +12045,32 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) is_using_builtin_array = true; if (var.basevariable && (var.basevariable == stage_in_ptr_var_id || var.basevariable == stage_out_ptr_var_id)) - decl += type_to_glsl(type, arg.id); + decl = join(cv_qualifier, type_to_glsl(type, arg.id)); else if (builtin) - decl += builtin_type_decl(builtin_type, arg.id); + decl = join(cv_qualifier, builtin_type_decl(builtin_type, arg.id)); else if ((storage == StorageClassUniform || storage == StorageClassStorageBuffer) && is_array(type)) { is_using_builtin_array = true; - decl += join(type_to_glsl(type, arg.id), "*"); + decl += join(cv_qualifier, type_to_glsl(type, arg.id), "*"); } else if (is_dynamic_img_sampler) { - decl += join("spvDynamicImageSampler<", type_to_glsl(get(type.image.type)), ">"); + decl = join(cv_qualifier, "spvDynamicImageSampler<", type_to_glsl(get(type.image.type)), ">"); // Mark the variable so that we can handle passing it to another function. set_extended_decoration(arg.id, SPIRVCrossDecorationDynamicImageSampler); } else - decl += type_to_glsl(type, arg.id); + { + // The type is a pointer type we need to emit cv_qualifier late. + if (type_is_pointer(type)) + { + decl = type_to_glsl(type, arg.id); + if (*cv_qualifier != '\0') + decl += join(" ", cv_qualifier); + } + else + decl = join(cv_qualifier, type_to_glsl(type, arg.id)); + } bool opaque_handle = storage == StorageClassUniformConstant; @@ -12099,8 +12176,12 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) // for the reference has to go before the '&', but after the '*'. if (!address_space.empty()) { - if (decl.back() == '*') - decl += join(" ", address_space, " "); + if (type_is_pointer(type)) + { + if (*cv_qualifier == '\0') + decl += ' '; + decl += join(address_space, " "); + } else decl = join(address_space, " ", decl); } @@ -12548,8 +12629,22 @@ string CompilerMSL::type_to_glsl(const SPIRType &type, uint32_t id) // Pointer? if (type.pointer) { + assert(type.pointer_depth > 0); + const char *restrict_kw; - type_name = join(get_type_address_space(type, id), " ", type_to_glsl(get(type.parent_type), id)); + + auto type_address_space = get_type_address_space(type, id); + auto type_decl = type_to_glsl(get(type.parent_type), id); + + // Work around C pointer qualifier rules. If glsl_type is a pointer type as well + // we'll need to emit the address space to the right. + // We could always go this route, but it makes the code unnatural. + // Prefer emitting thread T *foo over T thread* foo since it's more readable, + // but we'll have to emit thread T * thread * T constant bar; for example. + if (type_is_pointer_to_pointer(type)) + type_name = join(type_decl, " ", type_address_space, " "); + else + type_name = join(type_address_space, " ", type_decl); switch (type.basetype) { @@ -14912,7 +15007,7 @@ void CompilerMSL::analyze_argument_buffers() // Create a buffer to hold extra data, including the swizzle constants. SPIRType uint_type_pointer = get_uint_type(); uint_type_pointer.pointer = true; - uint_type_pointer.pointer_depth = 1; + uint_type_pointer.pointer_depth++; uint_type_pointer.parent_type = get_uint_type_id(); uint_type_pointer.storage = StorageClassUniform; set(uint_ptr_type_id, uint_type_pointer); @@ -14986,7 +15081,7 @@ void CompilerMSL::analyze_argument_buffers() auto &ptr_type = set(ptr_type_id); ptr_type = buffer_type; ptr_type.pointer = true; - ptr_type.pointer_depth = 1; + ptr_type.pointer_depth++; ptr_type.parent_type = type_id; uint32_t buffer_variable_id = next_id; @@ -15073,6 +15168,7 @@ void CompilerMSL::analyze_argument_buffers() set(atomic_type_id, atomic_type); atomic_type.pointer = true; + atomic_type.pointer_depth++; atomic_type.parent_type = atomic_type_id; atomic_type.storage = StorageClassStorageBuffer; auto &atomic_ptr_type = set(type_ptr_id, atomic_type); diff --git a/spirv_msl.hpp b/spirv_msl.hpp index 6efe453e..52e96761 100644 --- a/spirv_msl.hpp +++ b/spirv_msl.hpp @@ -1040,6 +1040,8 @@ protected: void activate_argument_buffer_resources(); bool type_is_msl_framebuffer_fetch(const SPIRType &type) const; + bool type_is_pointer(const SPIRType &type) const; + bool type_is_pointer_to_pointer(const SPIRType &type) const; bool is_supported_argument_buffer_type(const SPIRType &type) const; // OpcodeHandler that handles several MSL preprocessing operations.