From fadaec205b3a7fd19a08cd50d16a1f81954c23dd Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 13 Jan 2017 16:31:13 +0100 Subject: [PATCH 1/3] Improvements to legacy GLSL output. - By default, emit uniform structs for UBOs, like push constant. - Forward transpose information, and optimize transpose(matrix) * vector to vector * matrix. --- spirv_common.hpp | 4 +++ spirv_glsl.cpp | 84 ++++++++++++++++++++++++++++++++++++++++++------ spirv_glsl.hpp | 6 ++-- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/spirv_common.hpp b/spirv_common.hpp index 58ced976..e17e3534 100644 --- a/spirv_common.hpp +++ b/spirv_common.hpp @@ -351,6 +351,10 @@ struct SPIRExpression : IVariant // If this expression has been used while invalidated. bool used_while_invalidated = false; + // Before use, this expression must be transposed. + // This is needed for targets which don't support row_major layouts. + bool need_transpose = false; + // A list of expressions which this expression depends on. std::vector expression_dependencies; }; diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 95c80896..62ed6907 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -1000,12 +1000,36 @@ void CompilerGLSL::emit_push_constant_block_glsl(const SPIRVariable &var) statement(""); } +void CompilerGLSL::emit_buffer_block_legacy(const SPIRVariable &var) +{ + auto &type = get(var.basetype); + + // We're emitting the push constant block as a regular struct, so disable the block qualifier temporarily. + // Otherwise, we will end up emitting layout() qualifiers on naked structs which is not allowed. + auto &block_flags = meta[type.self].decoration.decoration_flags; + uint64_t block_flag = block_flags & (1ull << DecorationBlock); + block_flags &= ~block_flag; + emit_struct(type); + block_flags |= block_flag; + emit_uniform(var); + statement(""); +} + void CompilerGLSL::emit_buffer_block(const SPIRVariable &var) { auto &type = get(var.basetype); bool ssbo = (meta[type.self].decoration.decoration_flags & (1ull << DecorationBufferBlock)) != 0; bool is_restrict = (meta[var.self].decoration.decoration_flags & (1ull << DecorationRestrict)) != 0; + // By default, for legacy targets, fall back to declaring a uniform struct. + if (is_legacy()) + { + if (ssbo) + SPIRV_CROSS_THROW("SSBOs not supported in legacy targets."); + emit_buffer_block_legacy(var); + return; + } + add_resource_name(var.self); // Block names should never alias. @@ -1018,6 +1042,7 @@ void CompilerGLSL::emit_buffer_block(const SPIRVariable &var) else resource_names.insert(buffer_name); + statement(layout_for_variable(var), is_restrict ? "restrict " : "", ssbo ? "buffer " : "uniform ", buffer_name); begin_scope(); @@ -1055,6 +1080,9 @@ void CompilerGLSL::emit_interface_block(const SPIRVariable &var) if (block) { + if (is_legacy()) + SPIRV_CROSS_THROW("IO blocks are not supported in legacy targets."); + add_resource_name(var.self); // Block names should never alias. @@ -1574,6 +1602,8 @@ string CompilerGLSL::to_expression(uint32_t id) auto &e = get(id); if (e.base_expression) return to_enclosed_expression(e.base_expression) + e.expression; + else if (e.need_transpose) + return convert_row_major_matrix(e.expression); else return e.expression; } @@ -3097,7 +3127,7 @@ const char *CompilerGLSL::index_to_swizzle(uint32_t index) } string CompilerGLSL::access_chain(uint32_t base, const uint32_t *indices, uint32_t count, bool index_is_literal, - bool chain_only) + bool chain_only, bool *need_transpose) { string expr; if (!chain_only) @@ -3218,6 +3248,8 @@ string CompilerGLSL::access_chain(uint32_t base, const uint32_t *indices, uint32 SPIRV_CROSS_THROW("Cannot subdivide a scalar value!"); } + if (need_transpose) + *need_transpose = row_major_matrix_needs_conversion; return expr; } @@ -3554,13 +3586,28 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction) // If an expression is mutable and forwardable, we speculate that it is immutable. bool forward = should_forward(ptr) && forced_temporaries.find(id) == end(forced_temporaries); - // If loading a non-native row-major matrix, convert it to column-major + // If loading a non-native row-major matrix, mark the expression as need_transpose. + bool need_transpose = false; + bool old_need_transpose = false; + + auto *ptr_expression = maybe_get(ptr); + if (ptr_expression && ptr_expression->need_transpose) + { + old_need_transpose = true; + ptr_expression->need_transpose = false; + need_transpose = true; + } + else if (is_non_native_row_major_matrix(ptr)) + need_transpose = true; + auto expr = to_expression(ptr); - if (is_non_native_row_major_matrix(ptr)) - expr = convert_row_major_matrix(expr); + + if (ptr_expression) + ptr_expression->need_transpose = old_need_transpose; // Suppress usage tracking since using same expression multiple times does not imply any extra work. - emit_op(result_type, id, expr, forward, true); + auto &e = emit_op(result_type, id, expr, forward, true); + e.need_transpose = need_transpose; register_read(id, ptr, forward); break; } @@ -3574,9 +3621,11 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction) // If the base is immutable, the access chain pointer must also be. // If an expression is mutable and forwardable, we speculate that it is immutable. - auto e = access_chain(ops[2], &ops[3], length - 3, false); + bool need_transpose; + auto e = access_chain(ops[2], &ops[3], length - 3, false, false, &need_transpose); auto &expr = set(ops[1], move(e), ops[0], should_forward(ops[2])); expr.loaded_from = ops[2]; + expr.need_transpose = need_transpose; break; } @@ -4012,11 +4061,25 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction) break; } - case OpFMul: + case OpVectorTimesMatrix: case OpMatrixTimesVector: + { + // If the matrix needs transpose, just flip the multiply order. + auto *e = maybe_get(ops[opcode == OpMatrixTimesVector ? 2 : 3]); + if (e && e->need_transpose) + { + e->need_transpose = false; + emit_binary_op(ops[0], ops[1], ops[3], ops[2], "*"); + e->need_transpose = true; + } + else + BOP(*); + break; + } + + case OpFMul: case OpMatrixTimesScalar: case OpVectorTimesScalar: - case OpVectorTimesMatrix: case OpMatrixTimesMatrix: BOP(*); break; @@ -4855,7 +4918,8 @@ void CompilerGLSL::add_member_name(SPIRType &type, uint32_t index) bool CompilerGLSL::is_non_native_row_major_matrix(uint32_t id) { // Natively supported row-major matrices do not need to be converted. - if (backend.native_row_major_matrix) + // Legacy targets do not support row major. + if (backend.native_row_major_matrix && !is_legacy()) return false; // Non-matrix or column-major matrix types do not need to be converted. @@ -4876,7 +4940,7 @@ bool CompilerGLSL::is_non_native_row_major_matrix(uint32_t id) bool CompilerGLSL::member_is_non_native_row_major_matrix(const SPIRType &type, uint32_t index) { // Natively supported row-major matrices do not need to be converted. - if (backend.native_row_major_matrix) + if (backend.native_row_major_matrix && !is_legacy()) return false; // Non-matrix or column-major matrix types do not need to be converted. diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index b597f75f..7bd98f2a 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -264,6 +264,7 @@ protected: void emit_struct(SPIRType &type); void emit_resources(); void emit_buffer_block(const SPIRVariable &type); + void emit_buffer_block_legacy(const SPIRVariable &var); void emit_push_constant_block(const SPIRVariable &var); void emit_push_constant_block_vulkan(const SPIRVariable &var); void emit_push_constant_block_glsl(const SPIRVariable &var); @@ -304,8 +305,9 @@ protected: bool expression_is_forwarded(uint32_t id); SPIRExpression &emit_op(uint32_t result_type, uint32_t result_id, const std::string &rhs, bool forward_rhs, bool suppress_usage_tracking = false); - std::string access_chain(uint32_t base, const uint32_t *indices, uint32_t count, bool index_is_literal, - bool chain_only = false); + std::string access_chain(uint32_t base, const uint32_t *indices, uint32_t count, + bool index_is_literal, + bool chain_only = false, bool *need_transpose = nullptr); const char *index_to_swizzle(uint32_t index); std::string remap_swizzle(uint32_t result_type, uint32_t input_components, uint32_t expr); From 27f4f75513ec2a14e4090053bb6e7723d3623c73 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 13 Jan 2017 16:32:54 +0100 Subject: [PATCH 2/3] Run format_all.sh. --- main.cpp | 2 +- spirv_common.hpp | 2 +- spirv_glsl.cpp | 5 ++--- spirv_glsl.hpp | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/main.cpp b/main.cpp index ff79bcd1..ce658aec 100644 --- a/main.cpp +++ b/main.cpp @@ -27,7 +27,7 @@ #include #ifdef _MSC_VER -#pragma warning(disable: 4996) +#pragma warning(disable : 4996) #endif using namespace spv; diff --git a/spirv_common.hpp b/spirv_common.hpp index e17e3534..8af88180 100644 --- a/spirv_common.hpp +++ b/spirv_common.hpp @@ -105,7 +105,7 @@ inline std::string convert_to_string(T &&t) #ifdef _MSC_VER #pragma warning(push) -#pragma warning(disable: 4996) +#pragma warning(disable : 4996) #endif inline std::string convert_to_string(float t) diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 62ed6907..9833102d 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -1042,7 +1042,6 @@ void CompilerGLSL::emit_buffer_block(const SPIRVariable &var) else resource_names.insert(buffer_name); - statement(layout_for_variable(var), is_restrict ? "restrict " : "", ssbo ? "buffer " : "uniform ", buffer_name); begin_scope(); @@ -2579,8 +2578,8 @@ void CompilerGLSL::emit_texture_op(const Instruction &i) string expr; bool forward = false; - expr += to_function_name(img, imgtype, !!fetch, !!gather, !!proj, !!coffsets, (!!coffset || !!offset), (!!grad_x || !!grad_y), !!lod, - !!dref); + expr += to_function_name(img, imgtype, !!fetch, !!gather, !!proj, !!coffsets, (!!coffset || !!offset), + (!!grad_x || !!grad_y), !!lod, !!dref); expr += "("; expr += to_function_args(img, imgtype, fetch, gather, proj, coord, coord_components, dref, grad_x, grad_y, lod, coffset, offset, bias, comp, sample, &forward); diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index 7bd98f2a..0e88886d 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -305,8 +305,7 @@ protected: bool expression_is_forwarded(uint32_t id); SPIRExpression &emit_op(uint32_t result_type, uint32_t result_id, const std::string &rhs, bool forward_rhs, bool suppress_usage_tracking = false); - std::string access_chain(uint32_t base, const uint32_t *indices, uint32_t count, - bool index_is_literal, + std::string access_chain(uint32_t base, const uint32_t *indices, uint32_t count, bool index_is_literal, bool chain_only = false, bool *need_transpose = nullptr); const char *index_to_swizzle(uint32_t index); From 41f7e5b6a1ec1bb404402f240b7b69030fca11d0 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 13 Jan 2017 16:41:27 +0100 Subject: [PATCH 3/3] Add ability to have legacy-specific tests. --- .../shaders/legacy/vert/transpose.legacy.vert | 22 +++++++++++++++++++ shaders/legacy/vert/transpose.legacy.vert | 20 +++++++++++++++++ test_shaders.py | 16 ++++++++++---- 3 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 reference/shaders/legacy/vert/transpose.legacy.vert create mode 100644 shaders/legacy/vert/transpose.legacy.vert diff --git a/reference/shaders/legacy/vert/transpose.legacy.vert b/reference/shaders/legacy/vert/transpose.legacy.vert new file mode 100644 index 00000000..c73d1a11 --- /dev/null +++ b/reference/shaders/legacy/vert/transpose.legacy.vert @@ -0,0 +1,22 @@ +#version 100 + +struct Buffer +{ + mat4 MVPRowMajor; + mat4 MVPColMajor; + mat4 M; +}; + +uniform Buffer _13; + +attribute vec4 Position; + +void main() +{ + vec4 c0 = _13.M * (Position * _13.MVPRowMajor); + vec4 c1 = _13.M * (_13.MVPColMajor * Position); + vec4 c2 = _13.M * (_13.MVPRowMajor * Position); + vec4 c3 = _13.M * (Position * _13.MVPColMajor); + gl_Position = ((c0 + c1) + c2) + c3; +} + diff --git a/shaders/legacy/vert/transpose.legacy.vert b/shaders/legacy/vert/transpose.legacy.vert new file mode 100644 index 00000000..84f61826 --- /dev/null +++ b/shaders/legacy/vert/transpose.legacy.vert @@ -0,0 +1,20 @@ +#version 310 es + +uniform Buffer +{ + layout(row_major) mat4 MVPRowMajor; + layout(column_major) mat4 MVPColMajor; + mat4 M; +}; + +layout(location = 0) in vec4 Position; + +void main() +{ + vec4 c0 = M * (MVPRowMajor * Position); + vec4 c1 = M * (MVPColMajor * Position); + vec4 c2 = M * (Position * MVPRowMajor); + vec4 c3 = M * (Position * MVPColMajor); + gl_Position = c0 + c1 + c2 + c3; +} + diff --git a/test_shaders.py b/test_shaders.py index daac82a6..701dcad8 100755 --- a/test_shaders.py +++ b/test_shaders.py @@ -66,7 +66,7 @@ def validate_shader(shader, vulkan): else: subprocess.check_call(['glslangValidator', shader]) -def cross_compile(shader, vulkan, spirv, eliminate, invalid_spirv): +def cross_compile(shader, vulkan, spirv, eliminate, invalid_spirv, is_legacy): spirv_f, spirv_path = tempfile.mkstemp() glsl_f, glsl_path = tempfile.mkstemp(suffix = os.path.basename(shader)) os.close(spirv_f) @@ -84,11 +84,15 @@ def cross_compile(shader, vulkan, spirv, eliminate, invalid_spirv): if not invalid_spirv: subprocess.check_call(['spirv-val', spirv_path]) + legacy_cmd = [] + if is_legacy: + legacy_cmd = ['--version', '100', '--es'] + spirv_cross_path = './spirv-cross' if eliminate: - subprocess.check_call([spirv_cross_path, '--remove-unused-variables', '--entry', 'main', '--output', glsl_path, spirv_path]) + subprocess.check_call([spirv_cross_path, '--remove-unused-variables', '--entry', 'main', '--output', glsl_path, spirv_path] + legacy_cmd) else: - subprocess.check_call([spirv_cross_path, '--entry', 'main', '--output', glsl_path, spirv_path]) + subprocess.check_call([spirv_cross_path, '--entry', 'main', '--output', glsl_path, spirv_path] + legacy_cmd) # A shader might not be possible to make valid GLSL from, skip validation for this case. if (not ('nocompat' in glsl_path)) and (not spirv): @@ -171,6 +175,9 @@ def shader_is_spirv(shader): def shader_is_invalid_spirv(shader): return '.invalid.' in shader +def shader_is_legacy(shader): + return '.legacy.' in shader + def test_shader(stats, shader, update, keep): joined_path = os.path.join(shader[0], shader[1]) vulkan = shader_is_vulkan(shader[1]) @@ -178,9 +185,10 @@ def test_shader(stats, shader, update, keep): eliminate = shader_is_eliminate_dead_variables(shader[1]) is_spirv = shader_is_spirv(shader[1]) invalid_spirv = shader_is_invalid_spirv(shader[1]) + is_legacy = shader_is_legacy(shader[1]) print('Testing shader:', joined_path) - spirv, glsl, vulkan_glsl = cross_compile(joined_path, vulkan, is_spirv, eliminate, invalid_spirv) + spirv, glsl, vulkan_glsl = cross_compile(joined_path, vulkan, is_spirv, eliminate, invalid_spirv, is_legacy) # Only test GLSL stats if we have a shader following GL semantics. if stats and (not vulkan) and (not is_spirv) and (not desktop):