From 5d4bb687bc63310b54e42394b9934f21fdc884bf Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Mon, 3 Oct 2016 17:17:11 +0200 Subject: [PATCH] Support arrays which have spec constant size. The size of an array can be a specialization constant or a spec constant op. This complicates things quite a lot. Reflection becomes very painful in the presence of expressions instead of literals so add a new array which expresses this. It is unlikely that we will need to do accurate reflection of interface types which have specialization constant size. SSBOs and UBOs will for now throw exception if a dynamic size is used since it is very difficult to know the real size. --- .../shaders/vulkan/frag/spec-constant.vk.frag | 10 +- .../vulkan/frag/spec-constant.vk.frag.vk | 13 ++- shaders/vulkan/frag/spec-constant.vk.frag | 12 ++- spirv_common.hpp | 9 +- spirv_cpp.cpp | 14 +-- spirv_cross.cpp | 8 +- spirv_glsl.cpp | 102 +++++++++++------- spirv_glsl.hpp | 2 + spirv_msl.cpp | 3 +- 9 files changed, 122 insertions(+), 51 deletions(-) diff --git a/reference/shaders/vulkan/frag/spec-constant.vk.frag b/reference/shaders/vulkan/frag/spec-constant.vk.frag index 48fefe92..0785cfd5 100644 --- a/reference/shaders/vulkan/frag/spec-constant.vk.frag +++ b/reference/shaders/vulkan/frag/spec-constant.vk.frag @@ -2,6 +2,11 @@ precision mediump float; precision highp int; +struct Foo +{ + float elems[(4 + 2)]; +}; + layout(location = 0) out vec4 FragColor; void main() @@ -46,6 +51,9 @@ void main() mediump int c35 = int(false); mediump uint c36 = uint(false); float c37 = float(false); - FragColor = vec4((t0 + t1)); + float vec0[4][(3 + 3)]; + float vec1[(3 + 2)][(4 + 5)]; + Foo foo; + FragColor = (((vec4((t0 + t1)) + vec4(vec0[0][0])) + vec4(vec1[0][0])) + vec4(foo.elems[3])); } diff --git a/reference/shaders/vulkan/frag/spec-constant.vk.frag.vk b/reference/shaders/vulkan/frag/spec-constant.vk.frag.vk index bffd6bf7..f8e241be 100644 --- a/reference/shaders/vulkan/frag/spec-constant.vk.frag.vk +++ b/reference/shaders/vulkan/frag/spec-constant.vk.frag.vk @@ -2,7 +2,6 @@ precision mediump float; precision highp int; -layout(location = 0) out vec4 FragColor; layout(constant_id = 1) const float _9 = 1.0; layout(constant_id = 2) const float _11 = 2.0; layout(constant_id = 3) const int _16 = 3; @@ -12,6 +11,13 @@ layout(constant_id = 6) const uint _35 = 6u; layout(constant_id = 7) const bool _56 = false; layout(constant_id = 8) const bool _57 = true; +struct Foo +{ + float elems[(_25 + 2)]; +}; + +layout(location = 0) out vec4 FragColor; + void main() { float t0 = _9; @@ -54,6 +60,9 @@ void main() mediump int c35 = int(_56); mediump uint c36 = uint(_56); float c37 = float(_56); - FragColor = vec4((t0 + t1)); + float vec0[_25][(_16 + 3)]; + float vec1[(_16 + 2)][(_25 + 5)]; + Foo foo; + FragColor = (((vec4((t0 + t1)) + vec4(vec0[0][0])) + vec4(vec1[0][0])) + vec4(foo.elems[_16])); } diff --git a/shaders/vulkan/frag/spec-constant.vk.frag b/shaders/vulkan/frag/spec-constant.vk.frag index 3cb75da5..03b625bf 100644 --- a/shaders/vulkan/frag/spec-constant.vk.frag +++ b/shaders/vulkan/frag/spec-constant.vk.frag @@ -12,6 +12,11 @@ layout(constant_id = 7) const bool g = false; layout(constant_id = 8) const bool h = true; // glslang doesn't seem to support partial spec constants or composites yet, so only test the basics. +struct Foo +{ + float elems[d + 2]; +}; + void main() { float t0 = a; @@ -63,5 +68,10 @@ void main() uint c36 = uint(g); // bool -> uint float c37 = float(g); // bool -> float - FragColor = vec4(t0 + t1); + // Flexible sized arrays with spec constants and spec constant ops. + float vec0[d][c + 3]; + float vec1[c + 2][d + 5]; + + Foo foo; + FragColor = vec4(t0 + t1) + vec0[0][0] + vec1[0][0] + foo.elems[c]; } diff --git a/spirv_common.hpp b/spirv_common.hpp index 5c1accb5..0b06c69a 100644 --- a/spirv_common.hpp +++ b/spirv_common.hpp @@ -202,9 +202,16 @@ struct SPIRType : IVariant uint32_t vecsize = 1; uint32_t columns = 1; - // Arrays, suport array of arrays by having a vector of array sizes. + // Arrays, support array of arrays by having a vector of array sizes. std::vector array; + // Array elements can be either specialization constants or specialization ops. + // This array determines how to interpret the array size. + // If an element is true, the element is a literal, + // otherwise, it's an expression, which must be resolved on demand. + // The actual size is not really known until runtime. + std::vector array_size_literal; + // Pointers bool pointer = false; spv::StorageClass storage = spv::StorageClassGeneric; diff --git a/spirv_cpp.cpp b/spirv_cpp.cpp index cc4c05c0..ae1737f6 100644 --- a/spirv_cpp.cpp +++ b/spirv_cpp.cpp @@ -410,8 +410,8 @@ string CompilerCPP::argument_decl(const SPIRFunction::Parameter &arg) string variable_name = to_name(var.self); remap_variable_type_name(type, variable_name, base); - for (auto &array : type.array) - base = join("std::array<", base, ", ", array, ">"); + for (uint32_t i = 0; i < type.array.size(); i++) + base = join("std::array<", base, ", ", to_array_size(type, i), ">"); return join(constref ? "const " : "", base, " &", variable_name); } @@ -421,16 +421,18 @@ string CompilerCPP::variable_decl(const SPIRType &type, const string &name) string base = type_to_glsl(type); remap_variable_type_name(type, name, base); bool runtime = false; - for (auto &array : type.array) + + for (uint32_t i = 0; i < type.array.size(); i++) { - if (array) - base = join("std::array<", base, ", ", array, ">"); - else + auto &array = type.array[i]; + if (!array && type.array_size_literal[i]) { // Avoid using runtime arrays with std::array since this is undefined. // Runtime arrays cannot be passed around as values, so this is fine. runtime = true; } + else + base = join("std::array<", base, ", ", to_array_size(type, i), ">"); } base += ' '; return base + name + (runtime ? "[1]" : ""); diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 14c16a7b..69320a18 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -1265,7 +1265,12 @@ void Compiler::parse(const Instruction &instruction) auto &arraybase = set(id); arraybase = base; - arraybase.array.push_back(get(ops[2]).scalar()); + + auto *c = maybe_get(ops[2]); + bool literal = c && !c->specialization; + + arraybase.array_size_literal.push_back(literal); + arraybase.array.push_back(literal ? c->scalar() : ops[2]); // Do NOT set arraybase.self! break; } @@ -1279,6 +1284,7 @@ void Compiler::parse(const Instruction &instruction) arraybase = base; arraybase.array.push_back(0); + arraybase.array_size_literal.push_back(true); // Do NOT set arraybase.self! break; } diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index cd0ce97d..bda54384 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -729,6 +729,7 @@ uint32_t CompilerGLSL::type_to_std430_array_stride(const SPIRType &type, uint64_ // Array stride is equal to aligned size of the underlying type. SPIRType tmp = type; tmp.array.pop_back(); + tmp.array_size_literal.pop_back(); uint32_t size = type_to_std430_size(tmp, flags); uint32_t alignment = type_to_std430_alignment(tmp, flags); return (size + alignment - 1) & ~(alignment - 1); @@ -737,7 +738,7 @@ uint32_t CompilerGLSL::type_to_std430_array_stride(const SPIRType &type, uint64_ uint32_t CompilerGLSL::type_to_std430_size(const SPIRType &type, uint64_t flags) { if (!type.array.empty()) - return type.array.back() * type_to_std430_array_stride(type, flags); + return to_array_size_literal(type, type.array.size() - 1) * type_to_std430_array_stride(type, flags); const uint32_t base_alignment = type_to_std430_base_size(type); uint32_t size = 0; @@ -1215,6 +1216,34 @@ void CompilerGLSL::emit_resources() if (!pls_inputs.empty() || !pls_outputs.empty()) emit_pls(); + bool emitted = false; + + // If emitted Vulkan GLSL, + // emit specialization constants as actual floats, + // spec op expressions will redirect to the constant name. + // + // TODO: If we have the fringe case that we create a spec constant which depends on a struct type, + // we'll have to deal with that, but there's currently no known way to express that. + if (options.vulkan_semantics) + { + for (auto &id : ids) + { + if (id.get_type() == TypeConstant) + { + auto &c = id.get(); + if (!c.specialization) + continue; + + emit_specialization_constant(c); + emitted = true; + } + } + } + + if (emitted) + statement(""); + emitted = false; + // Output all basic struct types which are not Block or BufferBlock as these are declared inplace // when such variables are instantiated. for (auto &id : ids) @@ -1263,8 +1292,6 @@ void CompilerGLSL::emit_resources() } } - bool emitted = false; - bool skip_separate_image_sampler = !combined_image_samplers.empty() || !options.vulkan_semantics; // Output Uniform Constants (values, samplers, images, etc). @@ -1338,25 +1365,6 @@ void CompilerGLSL::emit_resources() } } - // If emitted Vulkan GLSL, - // emit specialization constants as actual floats, - // spec op expressions will redirect to the constant name. - if (options.vulkan_semantics) - { - for (auto &id : ids) - { - if (id.get_type() == TypeConstant) - { - auto &c = id.get(); - if (!c.specialization) - continue; - - emit_specialization_constant(c); - emitted = true; - } - } - } - if (emitted) statement(""); } @@ -4702,6 +4710,39 @@ string CompilerGLSL::pls_decl(const PlsRemap &var) to_name(variable.self)); } +uint32_t CompilerGLSL::to_array_size_literal(const SPIRType &type, uint32_t index) const +{ + assert(type.array.size() == type.array_size_literal.size()); + + if (!type.array_size_literal[index]) + throw CompilerError("The array size is not a literal, but a specialization constant or spec constant op."); + + return type.array[index]; +} + +string CompilerGLSL::to_array_size(const SPIRType &type, uint32_t index) +{ + assert(type.array.size() == type.array_size_literal.size()); + + auto &size = type.array[index]; + if (!type.array_size_literal[index]) + return to_expression(size); + else if (size) + return convert_to_string(size); + else if (!backend.flexible_member_array_supported) + { + // For runtime-sized arrays, we can work around + // lack of standard support for this by simply having + // a single element array. + // + // Runtime length arrays must always be the last element + // in an interface block. + return "1"; + } + else + return ""; +} + string CompilerGLSL::type_to_array_glsl(const SPIRType &type) { if (type.array.empty()) @@ -4710,23 +4751,8 @@ string CompilerGLSL::type_to_array_glsl(const SPIRType &type) string res; for (size_t i = type.array.size(); i; i--) { - auto &size = type.array[i - 1]; - res += "["; - if (size) - { - res += convert_to_string(size); - } - else if (!backend.flexible_member_array_supported) - { - // For runtime-sized arrays, we can work around - // lack of standard support for this by simply having - // a single element array. - // - // Runtime length arrays must always be the last element - // in an interface block. - res += '1'; - } + res += to_array_size(type, i - 1); res += "]"; } return res; diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index 252f68b4..fa948560 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -204,6 +204,8 @@ protected: Options options; std::string type_to_array_glsl(const SPIRType &type); + std::string to_array_size(const SPIRType &type, uint32_t index); + uint32_t to_array_size_literal(const SPIRType &type, uint32_t index) const; std::string variable_decl(const SPIRVariable &variable); void add_local_variable_name(uint32_t id); diff --git a/spirv_msl.cpp b/spirv_msl.cpp index a66b8a00..b5d5cd26 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -1260,6 +1260,7 @@ SPIRType &CompilerMSL::get_pad_type(uint32_t pad_len) ib_type.basetype = SPIRType::Char; ib_type.width = 8; ib_type.array.push_back(pad_len); + ib_type.array_size_literal.push_back(true); set_decoration(ib_type.self, DecorationArrayStride, pad_len); pad_type_ids_by_pad_len[pad_len] = pad_type_id; @@ -1616,7 +1617,7 @@ size_t CompilerMSL::get_declared_type_size(const SPIRType &type, uint64_t dec_ma // ArrayStride is part of the array type not OpMemberDecorate. auto &dec = meta[type.self].decoration; if (dec.decoration_flags & (1ull << DecorationArrayStride)) - return dec.array_stride * type.array.back(); + return dec.array_stride * to_array_size_literal(type, type.array.size() - 1); else throw CompilerError("Type does not have ArrayStride set."); }