From 6c41f9e9da9a4b61b66ae6109f400e41e5b6de68 Mon Sep 17 00:00:00 2001 From: Vadim Shcherbakov Date: Wed, 6 Dec 2017 09:51:23 -0800 Subject: [PATCH 1/4] MSL improvements: - pack/unpack nested constant buffer structs - support for write-only textures (only global ones for now) - better rt index support for msl generator --- spirv_common.hpp | 4 +++- spirv_cross.cpp | 7 +++++++ spirv_msl.cpp | 24 +++++++++++++++++++----- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/spirv_common.hpp b/spirv_common.hpp index caf13aab..eeb85325 100644 --- a/spirv_common.hpp +++ b/spirv_common.hpp @@ -657,10 +657,11 @@ struct SPIRVariable : IVariant }; SPIRVariable() = default; - SPIRVariable(uint32_t basetype_, spv::StorageClass storage_, uint32_t initializer_ = 0) + SPIRVariable(uint32_t basetype_, spv::StorageClass storage_, uint32_t initializer_ = 0, uint32_t basevariable_ = 0) : basetype(basetype_) , storage(storage_) , initializer(initializer_) + , basevariable(basevariable_) { } @@ -668,6 +669,7 @@ struct SPIRVariable : IVariant spv::StorageClass storage = spv::StorageClassGeneric; uint32_t decoration = 0; uint32_t initializer = 0; + uint32_t basevariable = 0; std::vector dereference_chain; bool compat_builtin = false; diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 6fdd732a..628ce45b 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -1706,6 +1706,13 @@ void Compiler::parse(const Instruction &instruction) auto &var = set(id, type, storage, initializer); + auto &ttype = get(type); + if (ttype.basetype == SPIRType::BaseType::Image) + { + set_decoration(id, DecorationNonWritable); + set_decoration(id, DecorationNonReadable); + } + if (variable_storage_is_aliased(var)) aliased_variables.push_back(var.self); diff --git a/spirv_msl.cpp b/spirv_msl.cpp index e3badb70..31af7e99 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -301,9 +301,10 @@ void CompilerMSL::extract_global_variables_from_function(uint32_t func_id, std:: uint32_t next_id = increase_bound_by(uint32_t(added_arg_ids.size())); for (uint32_t arg_id : added_arg_ids) { - uint32_t type_id = get(arg_id).basetype; + auto var = get(arg_id); + uint32_t type_id = var.basetype; func.add_parameter(type_id, next_id, true); - set(next_id, type_id, StorageClassFunction); + set(next_id, type_id, StorageClassFunction, 0, arg_id); // Ensure both the existing and new variables have the same name, and the name is valid string vld_name = ensure_valid_name(to_name(arg_id), "v"); @@ -360,6 +361,11 @@ void CompilerMSL::mark_as_packable(SPIRType &type) uint32_t mbr_type_id = type.member_types[mbr_idx]; auto &mbr_type = get(mbr_type_id); mark_as_packable(mbr_type); + if (mbr_type.type_alias) + { + auto &mbr_type_alias = get(mbr_type.type_alias); + mark_as_packable(mbr_type_alias); + } } } } @@ -1351,8 +1357,11 @@ void CompilerMSL::emit_instruction(const Instruction &instruction) // Mark that this shader reads from this image uint32_t img_id = ops[2]; auto *p_var = maybe_get_backing_variable(img_id); - if (p_var) + if (p_var && has_decoration(p_var->self, DecorationNonReadable)) + { unset_decoration(p_var->self, DecorationNonReadable); + force_recompile = true; + } emit_texture_op(instruction); break; @@ -2673,7 +2682,7 @@ string CompilerMSL::argument_decl(const SPIRFunction::Parameter &arg) if (constref) decl += "const "; - decl += type_to_glsl(type); + decl += type_to_glsl(type, arg.id); if (is_array(type)) decl += "*"; @@ -2940,6 +2949,8 @@ string CompilerMSL::image_type_glsl(const SPIRType &type, uint32_t id) default: { auto *p_var = maybe_get_backing_variable(id); + if (p_var && p_var->basevariable) + p_var = maybe_get(p_var->basevariable); if (p_var && !has_decoration(p_var->self, DecorationNonWritable)) { img_type_name += ", access::"; @@ -2996,12 +3007,13 @@ string CompilerMSL::builtin_to_glsl(BuiltIn builtin, StorageClass storage) return "gl_VertexIndex"; case BuiltInInstanceIndex: return "gl_InstanceIndex"; + case BuiltInLayer: + return current_function && (current_function->self == entry_point) ? stage_out_var_name + ".gl_Layer": "gl_Layer"; // When used in the entry function, output builtins are qualified with output struct name. case BuiltInPosition: case BuiltInPointSize: case BuiltInClipDistance: - case BuiltInLayer: case BuiltInFragDepth: if (current_function && (current_function->self == entry_point)) return stage_out_var_name + "." + CompilerGLSL::builtin_to_glsl(builtin, storage); @@ -3096,6 +3108,8 @@ string CompilerMSL::builtin_type_decl(BuiltIn builtin) return "uint"; case BuiltInInstanceIndex: return "uint"; + case BuiltInLayer: + return "uint"; // Vertex function out case BuiltInClipDistance: From 717d9fefd851ab9f04ffae1ae088b2ef755c49c6 Mon Sep 17 00:00:00 2001 From: Vadim Shcherbakov Date: Mon, 11 Dec 2017 21:02:13 +0300 Subject: [PATCH 2/4] another formatting fix and a comment --- spirv_cross.cpp | 1 + spirv_msl.cpp | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 628ce45b..f6504d13 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -1706,6 +1706,7 @@ void Compiler::parse(const Instruction &instruction) auto &var = set(id, type, storage, initializer); + // hlsl based shaders don't have those decorations. force them and then reset when reading/writing images auto &ttype = get(type); if (ttype.basetype == SPIRType::BaseType::Image) { diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 31af7e99..9c36c70a 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -3008,7 +3008,7 @@ string CompilerMSL::builtin_to_glsl(BuiltIn builtin, StorageClass storage) case BuiltInInstanceIndex: return "gl_InstanceIndex"; case BuiltInLayer: - return current_function && (current_function->self == entry_point) ? stage_out_var_name + ".gl_Layer": "gl_Layer"; + return current_function && (current_function->self == entry_point) ? stage_out_var_name + ".gl_Layer": "gl_Layer"; // When used in the entry function, output builtins are qualified with output struct name. case BuiltInPosition: @@ -3109,7 +3109,7 @@ string CompilerMSL::builtin_type_decl(BuiltIn builtin) case BuiltInInstanceIndex: return "uint"; case BuiltInLayer: - return "uint"; + return "uint"; // Vertex function out case BuiltInClipDistance: From db402236a75d7453f76a3c76d22f58b01368df50 Mon Sep 17 00:00:00 2001 From: Vadim Shcherbakov Date: Wed, 13 Dec 2017 13:02:03 +0300 Subject: [PATCH 3/4] move BuiltInLayer to vertex out function block --- spirv_msl.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spirv_msl.cpp b/spirv_msl.cpp index 9c36c70a..e8f6b2a1 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -3007,14 +3007,13 @@ string CompilerMSL::builtin_to_glsl(BuiltIn builtin, StorageClass storage) return "gl_VertexIndex"; case BuiltInInstanceIndex: return "gl_InstanceIndex"; - case BuiltInLayer: - return current_function && (current_function->self == entry_point) ? stage_out_var_name + ".gl_Layer": "gl_Layer"; // When used in the entry function, output builtins are qualified with output struct name. case BuiltInPosition: case BuiltInPointSize: case BuiltInClipDistance: case BuiltInFragDepth: + case BuiltInLayer: if (current_function && (current_function->self == entry_point)) return stage_out_var_name + "." + CompilerGLSL::builtin_to_glsl(builtin, storage); else @@ -3108,8 +3107,6 @@ string CompilerMSL::builtin_type_decl(BuiltIn builtin) return "uint"; case BuiltInInstanceIndex: return "uint"; - case BuiltInLayer: - return "uint"; // Vertex function out case BuiltInClipDistance: @@ -3118,6 +3115,8 @@ string CompilerMSL::builtin_type_decl(BuiltIn builtin) return "float"; case BuiltInPosition: return "float4"; + case BuiltInLayer: + return "uint"; // Fragment function in case BuiltInFrontFacing: From 3376198740f61573fcc0c065cd24476dbed33fce Mon Sep 17 00:00:00 2001 From: Vadim Shcherbakov Date: Wed, 13 Dec 2017 13:03:31 +0300 Subject: [PATCH 4/4] and a bit better case placemenent --- spirv_msl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spirv_msl.cpp b/spirv_msl.cpp index e8f6b2a1..087c4150 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -3012,8 +3012,8 @@ string CompilerMSL::builtin_to_glsl(BuiltIn builtin, StorageClass storage) case BuiltInPosition: case BuiltInPointSize: case BuiltInClipDistance: - case BuiltInFragDepth: case BuiltInLayer: + case BuiltInFragDepth: if (current_function && (current_function->self == entry_point)) return stage_out_var_name + "." + CompilerGLSL::builtin_to_glsl(builtin, storage); else