From 4f07a32c298a592f282c2072e41a67a6724f9a39 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Thu, 15 Dec 2016 17:14:47 +0100 Subject: [PATCH 1/7] Begin implementing for loop initializer propagation. --- .clang-format | 3 ++ spirv_cfg.hpp | 18 +++++++++ spirv_common.hpp | 14 +++++++ spirv_cross.cpp | 95 +++++++++++++++++++++++++++++++++++++++++++++++ spirv_glsl.cpp | 97 +++++++++++++++++++++++++++++++++++++++--------- spirv_glsl.hpp | 2 + 6 files changed, 211 insertions(+), 18 deletions(-) diff --git a/.clang-format b/.clang-format index 9b694ec6..443f90b7 100755 --- a/.clang-format +++ b/.clang-format @@ -162,3 +162,6 @@ TabWidth: 4 # The way to use tab characters in the resulting file. Possible values: Never, ForIndentation, Always. UseTab: ForIndentation + +# Do not reflow comments +ReflowComments: false diff --git a/spirv_cfg.hpp b/spirv_cfg.hpp index 07eb1196..e6450a71 100644 --- a/spirv_cfg.hpp +++ b/spirv_cfg.hpp @@ -56,6 +56,24 @@ public: uint32_t find_common_dominator(uint32_t a, uint32_t b) const; + const std::vector &get_preceding_edges(uint32_t block) const + { + return preceding_edges[block]; + } + + const std::vector &get_succeeding_edges(uint32_t block) const + { + return succeeding_edges[block]; + } + + template + void walk_from(uint32_t block, const Op &op) const + { + op(block); + for (auto b : succeeding_edges[block]) + walk_from(b, op); + } + private: Compiler &compiler; const SPIRFunction &func; diff --git a/spirv_common.hpp b/spirv_common.hpp index 4f0b0d0b..bfc6ad2b 100644 --- a/spirv_common.hpp +++ b/spirv_common.hpp @@ -445,6 +445,11 @@ struct SPIRBlock : IVariant // All access to these variables are dominated by this block, // so before branching anywhere we need to make sure that we declare these variables. std::vector dominated_variables; + + // These are variables which should be declared in a for loop header, if we + // fail to use a classic for-loop, + // we remove these variables, and fall back to regular variables outside the loop. + std::vector loop_variables; }; struct SPIRFunction : IVariant @@ -553,6 +558,15 @@ struct SPIRVariable : IVariant bool remapped_variable = false; uint32_t remapped_components = 0; + // The block which dominates all access to this variable. + uint32_t dominator = 0; + // If true, this variable is a loop variable, when accessing the variable + // outside a loop, + // we should statically forward it. + bool loop_variable = false; + // Set to true while we're inside the for loop. + bool loop_variable_enable = false; + SPIRFunction::Parameter *parameter = nullptr; }; diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 10dd31c9..64a2ae39 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -2909,6 +2909,8 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry) // Compute the control flow graph for this function. CFG cfg(*this, entry); + unordered_map potential_loop_variables; + // For each variable which is statically accessed. for (auto &var : handler.accessed_variables_to_block) { @@ -2917,7 +2919,22 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry) // Figure out which block is dominating all accesses of those variables. for (auto &block : blocks) + { + // If we're accessing a variable inside a continue block, this variable + // might be a loop variable. + if (is_continue(block)) + { + // The variable is used in multiple continue blocks, this is not a loop + // candidate, signal that by setting block to -1u. + auto &potential = potential_loop_variables[var.first]; + + if (potential == 0) + potential = block; + else + potential = -1u; + } builder.add_block(block); + } builder.lift_continue_block_dominator(); @@ -2929,6 +2946,84 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry) { auto &block = this->get(dominating_block); block.dominated_variables.push_back(var.first); + get(var.first).dominator = dominating_block; } } + + // Now, try to analyze whether or not these variables are actually loop variables. + for (auto &loop_variable : potential_loop_variables) + { + auto &var = get(loop_variable.first); + auto dominator = var.dominator; + auto block = loop_variable.second; + + // The variable was accessed in multiple continue blocks, ignore. + if (block == -1u || block == 0) + continue; + + // Dead code. + if (dominator == 0) + continue; + + uint32_t header = 0; + + // Find the loop header for this block. + for (auto b : loop_blocks) + { + auto &potential_header = get(b); + if (potential_header.continue_block == block) + { + header = b; + break; + } + } + + assert(header); + auto &header_block = get(header); + + // Now, there are two conditions we need to meet for the variable to be a loop variable. + // 1. The dominating block must have a branch-free path to the loop header, + // this way we statically know which expression should be part of the loop variable initializer. + + // Walk from the dominator, if there is one straight edge connecting + // dominator and loop header, we statically know the loop initializer. + bool static_loop_init = true; + while (dominator != header) + { + auto &succ = cfg.get_succeeding_edges(dominator); + if (succ.size() != 1) + { + static_loop_init = false; + break; + } + + auto &pred = cfg.get_preceding_edges(succ.front()); + if (pred.size() != 1 || pred.front() != dominator) + { + static_loop_init = false; + break; + } + + dominator = succ.front(); + } + + if (!static_loop_init) + continue; + + // The second condition we need to meet is that no access after the loop + // merge can occur. Walk the CFG to see if we find anything. + auto &blocks = handler.accessed_variables_to_block[loop_variable.first]; + cfg.walk_from(header_block.merge_block, [&](uint32_t walk_block) { + // We found a block which accesses the variable outside the loop. + if (blocks.find(walk_block) != end(blocks)) + static_loop_init = false; + }); + + if (!static_loop_init) + continue; + + // We have a loop variable. + header_block.loop_variables.push_back(loop_variable.first); + get(loop_variable.first).loop_variable = true; + } } diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 862f1983..4422ecf6 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -3490,6 +3490,8 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction) if (var && var->statically_assigned) var->static_expression = ops[1]; + else if (var && var->loop_variable && !var->loop_variable_enable) + var->static_expression = ops[1]; else { auto lhs = to_expression(ops[0]); @@ -4920,7 +4922,9 @@ string CompilerGLSL::variable_decl(const SPIRVariable &variable) // Ignore the pointer type since GLSL doesn't have pointers. auto &type = get(variable.basetype); auto res = join(to_qualifiers_glsl(variable.self), variable_decl(type, to_name(variable.self))); - if (variable.initializer) + if (variable.loop_variable) + res += join(" = ", to_expression(variable.static_expression)); + else if (variable.initializer) res += join(" = ", to_expression(variable.initializer)); return res; } @@ -5345,6 +5349,16 @@ void CompilerGLSL::emit_function(SPIRFunction &func, uint64_t return_flags) begin_scope(); current_function = &func; + auto &entry_block = get(func.entry_block); + + if (!func.analyzed_variable_scope) + { + if (options.cfg_analysis) + analyze_variable_scope(func); + else + entry_block.dominated_variables = func.local_variables; + func.analyzed_variable_scope = true; + } for (auto &v : func.local_variables) { @@ -5365,24 +5379,19 @@ void CompilerGLSL::emit_function(SPIRFunction &func, uint64_t return_flags) } else { - // HACK: SPIRV likes to use samplers and images as local variables, but GLSL does not allow - // this. For these types (non-lvalue), we enforce forwarding through a shadowed variable. + // HACK: SPIRV likes to use samplers and images as local variables, but GLSL does not allow this. + // For these types (non-lvalue), we enforce forwarding through a shadowed variable. // This means that when we OpStore to these variables, we just write in the expression ID directly. // This breaks any kind of branching, since the variable must be statically assigned. // Branching on samplers and images would be pretty much impossible to fake in GLSL. var.statically_assigned = true; } - } - auto &entry_block = get(func.entry_block); + var.loop_variable_enable = false; - if (!func.analyzed_variable_scope) - { - if (options.cfg_analysis) - analyze_variable_scope(func); - else - entry_block.dominated_variables = func.local_variables; - func.analyzed_variable_scope = true; + // Loop variables are never declared outside their for-loop, so block any implicit declaration. + if (var.loop_variable) + var.deferred_declaration = false; } entry_block.loop_dominator = SPIRBlock::NoDominator; @@ -5612,6 +5621,36 @@ string CompilerGLSL::emit_continue_block(uint32_t continue_block) return merge(statements); } +string CompilerGLSL::emit_for_loop_initializers(const SPIRBlock &block) +{ + if (block.loop_variables.empty()) + return ""; + + if (block.loop_variables.size() == 1) + { + return variable_decl(get(block.loop_variables.front())); + } + else + { + auto &var = get(block.loop_variables.front()); + auto &type = get(var.basetype); + + // Don't remap the type here as we have multiple names, + // doesn't make sense to remap types for loop variables anyways. + // It is assumed here that all relevant qualifiers are equal for all loop variables. + string expr = join(to_qualifiers_glsl(var.self), type_to_glsl(type), " "); + + for (auto &loop_var : block.loop_variables) + { + auto &v = get(loop_var); + expr += join(to_name(loop_var), " = ", to_expression(v.static_expression)); + if (&loop_var != &block.loop_variables.back()) + expr += ", "; + } + return expr; + } +} + bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method method) { SPIRBlock::ContinueBlockType continue_type = continue_block_type(get(block.continue_block)); @@ -5633,8 +5672,12 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method switch (continue_type) { case SPIRBlock::ForLoop: - statement("for (; ", to_expression(block.condition), "; ", emit_continue_block(block.continue_block), - ")"); + // If we have loop variables, stop masking out access to the variable now. + for (auto var : block.loop_variables) + get(var).loop_variable_enable = true; + + statement("for (", emit_for_loop_initializers(block), "; ", to_expression(block.condition), "; ", + emit_continue_block(block.continue_block), ")"); break; case SPIRBlock::WhileLoop: @@ -5680,8 +5723,12 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method switch (continue_type) { case SPIRBlock::ForLoop: - statement("for (; ", to_expression(child.condition), "; ", emit_continue_block(block.continue_block), - ")"); + // If we have loop variables, stop masking out access to the variable now. + for (auto var : block.loop_variables) + get(var).loop_variable_enable = true; + + statement("for (", emit_for_loop_initializers(block), "; ", to_expression(child.condition), "; ", + emit_continue_block(block.continue_block), ")"); break; case SPIRBlock::WhileLoop: @@ -5725,6 +5772,7 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) bool select_branch_to_true_block = false; bool skip_direct_branch = false; + bool emitted_for_loop_header = false; // If we need to force temporaries for certain IDs due to continue blocks, do it before starting loop header. for (auto &tmp : block.declare_temporary) @@ -5744,9 +5792,9 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) flush_undeclared_variables(block); if (attempt_emit_loop_header(block, SPIRBlock::MergeToSelectForLoop)) { - // The body of while, is actually just the true block, so always branch there - // unconditionally. + // The body of while, is actually just the true block, so always branch there unconditionally. select_branch_to_true_block = true; + emitted_for_loop_header = true; } } // This is the newer loop behavior in glslang which branches from Loop header directly to @@ -5755,7 +5803,10 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) { flush_undeclared_variables(block); if (attempt_emit_loop_header(block, SPIRBlock::MergeToDirectForLoop)) + { skip_direct_branch = true; + emitted_for_loop_header = true; + } } else if (continue_type == SPIRBlock::DoWhileLoop) { @@ -5783,6 +5834,16 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) emit_instruction(op); } + // If we didn't successfully emit a loop header and we had loop variable candidates, we have a problem + // as writes to said loop variables might have been masked out, we need a recompile. + if (!emitted_for_loop_header && !block.loop_variables.empty()) + { + force_recompile = true; + for (auto var : block.loop_variables) + get(var).loop_variable = false; + block.loop_variables.clear(); + } + flush_undeclared_variables(block); bool emit_next_block = true; diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index 23c1bc3a..d6e5a177 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -388,6 +388,8 @@ protected: void check_function_call_constraints(const uint32_t *args, uint32_t length); void handle_invalid_expression(uint32_t id); void find_static_extensions(); + + std::string emit_for_loop_initializers(const SPIRBlock &block); }; } From 51d45511a653f5b2c21730e328239dc25db6e3c7 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Thu, 15 Dec 2016 17:54:49 +0100 Subject: [PATCH 2/7] Check if we can use multiple initializers. Need same type and qualifiers in GLSL and friends. --- reference/shaders/comp/cfg.comp | 3 +-- reference/shaders/comp/loop.comp | 6 ++--- reference/shaders/comp/return.comp | 3 +-- reference/shaders/comp/torture-loop.comp | 6 ++--- spirv_glsl.cpp | 31 ++++++++++++++++++++++++ 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/reference/shaders/comp/cfg.comp b/reference/shaders/comp/cfg.comp index 63d95ab1..56e4b06e 100644 --- a/reference/shaders/comp/cfg.comp +++ b/reference/shaders/comp/cfg.comp @@ -63,9 +63,8 @@ void test() break; } } - int i = 0; float h; - for (; i < 20; i = i + 1, h = h + 10.0) + for (int i = 0; i < 20; i = i + 1, h = h + 10.0) { } _11.data = h; diff --git a/reference/shaders/comp/loop.comp b/reference/shaders/comp/loop.comp index a9848dda..530ea277 100644 --- a/reference/shaders/comp/loop.comp +++ b/reference/shaders/comp/loop.comp @@ -60,11 +60,9 @@ void main() idat = idat * 2.0; k = k + 1; } - uint i_1 = 0u; - for (; i_1 < 16u; i_1 = i_1 + uint(1), k = k + 1) + for (uint i_1 = 0u; i_1 < 16u; i_1 = i_1 + uint(1), k = k + 1) { - uint j = 0u; - for (; j < 30u; j = j + uint(1)) + for (uint j = 0u; j < 30u; j = j + uint(1)) { idat = _24.mvp * idat; } diff --git a/reference/shaders/comp/return.comp b/reference/shaders/comp/return.comp index ebbaac2d..d23454c9 100644 --- a/reference/shaders/comp/return.comp +++ b/reference/shaders/comp/return.comp @@ -21,8 +21,7 @@ void main() return; } } - int i = 0; - for (; i < 20; i = i + 1) + for (int i = 0; i < 20; i = i + 1) { if (i == 10) { diff --git a/reference/shaders/comp/torture-loop.comp b/reference/shaders/comp/torture-loop.comp index abcbb7c5..23afa073 100644 --- a/reference/shaders/comp/torture-loop.comp +++ b/reference/shaders/comp/torture-loop.comp @@ -33,11 +33,9 @@ void main() break; } } - uint i = 0u; - for (; i < 16u; i = i + uint(1), k = k + 1) + for (uint i = 0u; i < 16u; i = i + uint(1), k = k + 1) { - uint j = 0u; - for (; j < 30u; j = j + uint(1)) + for (uint j = 0u; j < 30u; j = j + uint(1)) { idat = _24.mvp * idat; } diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 4422ecf6..2da2f4a8 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -5354,7 +5354,38 @@ void CompilerGLSL::emit_function(SPIRFunction &func, uint64_t return_flags) if (!func.analyzed_variable_scope) { if (options.cfg_analysis) + { analyze_variable_scope(func); + + // Check if we can actually use the loop variables we found in analyze_variable_scope. + // To use multiple initializers, we need the same type and qualifiers. + for (auto block : func.blocks) + { + auto &b = get(block); + if (b.loop_variables.size() < 2) + continue; + + uint64_t flags = get_decoration_mask(b.loop_variables.front()); + uint32_t type = get(b.loop_variables.front()).basetype; + bool invalid_initializers = false; + for (auto loop_variable : b.loop_variables) + { + if (flags != get_decoration_mask(loop_variable) || + type != get(b.loop_variables.front()).basetype) + { + invalid_initializers = true; + break; + } + } + + if (invalid_initializers) + { + for (auto loop_variable : b.loop_variables) + get(loop_variable).loop_variable = false; + b.loop_variables.clear(); + } + } + } else entry_block.dominated_variables = func.local_variables; func.analyzed_variable_scope = true; From a714d424d05a2f3f05b9b70039a81f1094857115 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 16 Dec 2016 12:43:12 +0100 Subject: [PATCH 3/7] Add directed test for for-loop-init. --- reference/shaders/frag/for-loop-init.frag | 52 +++++++++++++++++++++++ shaders/frag/for-loop-init.frag | 52 +++++++++++++++++++++++ spirv_cross.cpp | 7 +-- spirv_glsl.cpp | 16 +++---- 4 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 reference/shaders/frag/for-loop-init.frag create mode 100644 shaders/frag/for-loop-init.frag diff --git a/reference/shaders/frag/for-loop-init.frag b/reference/shaders/frag/for-loop-init.frag new file mode 100644 index 00000000..81161076 --- /dev/null +++ b/reference/shaders/frag/for-loop-init.frag @@ -0,0 +1,52 @@ +#version 310 es +precision mediump float; +precision highp int; + +layout(location = 0) out mediump int FragColor; + +void main() +{ + FragColor = 15; + for (mediump int i = 0; i < 25; i = i + 1) + { + FragColor = FragColor + 10; + } + for (mediump int j = 4, i_1 = 1; i_1 < 30; i_1 = i_1 + 1, j = j + 4) + { + FragColor = FragColor + 11; + } + mediump int k = 0; + for (; k < 20; k = k + 1) + { + FragColor = FragColor + 12; + } + k = k + 3; + FragColor = FragColor + k; + mediump int l; + if (k == 40) + { + l = 0; + for (; l < 40; l = l + 1) + { + FragColor = FragColor + 13; + } + return; + } + else + { + l = k; + FragColor = FragColor + l; + } + mediump ivec2 i_2 = ivec2(0); + for (; i_2.x < 10; i_2.x = i_2.x + 1) + { + FragColor = FragColor + i_2.y; + } + mediump int o = k; + for (mediump int m = k; m < 40; m = m + 1) + { + FragColor = FragColor + m; + } + FragColor = FragColor + o; +} + diff --git a/shaders/frag/for-loop-init.frag b/shaders/frag/for-loop-init.frag new file mode 100644 index 00000000..78b8903e --- /dev/null +++ b/shaders/frag/for-loop-init.frag @@ -0,0 +1,52 @@ +#version 310 es +precision mediump float; +layout(location = 0) out int FragColor; + +void main() +{ + FragColor = 15; + + // Basic loop variable. + for (int i = 0; i < 25; i++) + FragColor += 10; + + // Multiple loop variables. + for (int i = 1, j = 4; i < 30; i++, j += 4) + FragColor += 11; + + // A potential loop variables, but we access it outside the loop, + // so cannot be one. + int k = 0; + for (; k < 20; k++) + FragColor += 12; + k += 3; + FragColor += k; + + // Potential loop variables, but the dominator is not trivial. + int l; + if (k == 40) + { + for (l = 0; l < 40; l++) + FragColor += 13; + return; + } + else + { + l = k; + FragColor += l; + } + + // Vectors cannot be loop variables + for (ivec2 i = ivec2(0); i.x < 10; i.x++) + { + FragColor += i.y; + } + + // Check that static expressions can be used before the loop header. + int m = 0; + m = k; + int o = m; + for (; m < 40; m++) + FragColor += m; + FragColor += o; +} diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 64a2ae39..39ea21f6 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -2916,13 +2916,14 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry) { DominatorBuilder builder(cfg); auto &blocks = var.second; + auto &type = expression_type(var.first); // Figure out which block is dominating all accesses of those variables. for (auto &block : blocks) { - // If we're accessing a variable inside a continue block, this variable - // might be a loop variable. - if (is_continue(block)) + // If we're accessing a variable inside a continue block, this variable might be a loop variable. + // We can only use loop variables with scalars, as we cannot track static expressions for vectors. + if (is_continue(block) && type.vecsize == 1 && type.columns == 1) { // The variable is used in multiple continue blocks, this is not a loop // candidate, signal that by setting block to -1u. diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 2da2f4a8..e669078b 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -1565,7 +1565,9 @@ string CompilerGLSL::to_expression(uint32_t id) case TypeVariable: { auto &var = get(id); - if (var.statically_assigned) + // If we try to use a loop variable before the loop header, we have to redirect it to the static expression, + // the variable has not been declared yet. + if (var.statically_assigned || (var.loop_variable && !var.loop_variable_enable)) return to_expression(var.static_expression); else if (var.deferred_declaration) { @@ -5703,10 +5705,6 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method switch (continue_type) { case SPIRBlock::ForLoop: - // If we have loop variables, stop masking out access to the variable now. - for (auto var : block.loop_variables) - get(var).loop_variable_enable = true; - statement("for (", emit_for_loop_initializers(block), "; ", to_expression(block.condition), "; ", emit_continue_block(block.continue_block), ")"); break; @@ -5754,10 +5752,6 @@ bool CompilerGLSL::attempt_emit_loop_header(SPIRBlock &block, SPIRBlock::Method switch (continue_type) { case SPIRBlock::ForLoop: - // If we have loop variables, stop masking out access to the variable now. - for (auto var : block.loop_variables) - get(var).loop_variable_enable = true; - statement("for (", emit_for_loop_initializers(block), "; ", to_expression(child.condition), "; ", emit_continue_block(block.continue_block), ")"); break; @@ -5817,6 +5811,10 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) if (block.continue_block) continue_type = continue_block_type(get(block.continue_block)); + // If we have loop variables, stop masking out access to the variable now. + for (auto var : block.loop_variables) + get(var).loop_variable_enable = true; + // This is the older loop behavior in glslang which branches to loop body directly from the loop header. if (block_is_loop_candidate(block, SPIRBlock::MergeToSelectForLoop)) { From 62613df5a5c2f8846025a290267868f4f0d4ff6a Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 16 Dec 2016 13:14:22 +0100 Subject: [PATCH 4/7] Optimize for read-modify-writes. Required for legacy loop increments. --- reference/shaders/comp/cfg.comp | 2 +- reference/shaders/comp/dowhile.comp | 2 +- .../shaders/comp/inout-struct.invalid.comp | 8 ++--- reference/shaders/comp/loop.comp | 32 +++++++++---------- reference/shaders/comp/return.comp | 2 +- reference/shaders/comp/torture-loop.comp | 10 +++--- .../desktop-only/comp/fp64.desktop.comp | 10 +++--- .../desktop-only/comp/int64.desktop.comp | 18 +++++------ reference/shaders/frag/for-loop-init.frag | 32 +++++++++---------- reference/shaders/frag/ground.frag | 2 +- reference/shaders/tese/water_tess.tese | 4 +-- reference/shaders/vert/ground.vert | 4 +-- reference/shaders/vert/ocean.vert | 4 +-- spirv_glsl.cpp | 24 +++++++++++++- spirv_glsl.hpp | 2 ++ 15 files changed, 90 insertions(+), 66 deletions(-) diff --git a/reference/shaders/comp/cfg.comp b/reference/shaders/comp/cfg.comp index 56e4b06e..ed4a45c8 100644 --- a/reference/shaders/comp/cfg.comp +++ b/reference/shaders/comp/cfg.comp @@ -64,7 +64,7 @@ void test() } } float h; - for (int i = 0; i < 20; i = i + 1, h = h + 10.0) + for (int i = 0; i < 20; i += 1, h += 10.0) { } _11.data = h; diff --git a/reference/shaders/comp/dowhile.comp b/reference/shaders/comp/dowhile.comp index 01449340..dabf08f5 100644 --- a/reference/shaders/comp/dowhile.comp +++ b/reference/shaders/comp/dowhile.comp @@ -22,7 +22,7 @@ void main() do { idat = _28.mvp * idat; - i = i + 1; + i += 1; } while (i < 16); _52.out_data[ident] = idat; } diff --git a/reference/shaders/comp/inout-struct.invalid.comp b/reference/shaders/comp/inout-struct.invalid.comp index d636dd64..1aaa48f2 100644 --- a/reference/shaders/comp/inout-struct.invalid.comp +++ b/reference/shaders/comp/inout-struct.invalid.comp @@ -35,10 +35,10 @@ void baz(out Foo foo) void meow(inout Foo foo) { - foo.a = foo.a + vec4(10.0); - foo.b = foo.b + vec4(20.0); - foo.c = foo.c + vec4(30.0); - foo.d = foo.d + vec4(40.0); + foo.a += vec4(10.0); + foo.b += vec4(20.0); + foo.c += vec4(30.0); + foo.d += vec4(40.0); } vec4 bar(Foo foo) diff --git a/reference/shaders/comp/loop.comp b/reference/shaders/comp/loop.comp index 530ea277..fbc7d4a0 100644 --- a/reference/shaders/comp/loop.comp +++ b/reference/shaders/comp/loop.comp @@ -22,8 +22,8 @@ void main() { do { - k = k * 2; - i = i + uint(1); + k *= 2; + i += uint(1); } while (i < ident); } switch (k) @@ -32,7 +32,7 @@ void main() { for (;;) { - i = i + uint(1); + i += uint(1); if (i > 10u) { break; @@ -45,7 +45,7 @@ void main() { for (;;) { - i = i + 2u; + i += 2u; if (i > 20u) { break; @@ -57,12 +57,12 @@ void main() } while (k < 10) { - idat = idat * 2.0; - k = k + 1; + idat *= 2.0; + k += 1; } - for (uint i_1 = 0u; i_1 < 16u; i_1 = i_1 + uint(1), k = k + 1) + for (uint i_1 = 0u; i_1 < 16u; i_1 += uint(1), k += 1) { - for (uint j = 0u; j < 30u; j = j + uint(1)) + for (uint j = 0u; j < 30u; j += uint(1)) { idat = _24.mvp * idat; } @@ -70,34 +70,34 @@ void main() k = 0; for (;;) { - k = k + 1; + k += 1; if (k > 10) { - k = k + 2; + k += 2; } else { - k = k + 3; + k += 3; continue; } - k = k + 10; + k += 10; continue; } k = 0; do { - k = k + 1; + k += 1; } while (k > 10); int l = 0; for (;;) { if (l == 5) { - l = l + 1; + l += 1; continue; } - idat = idat + vec4(1.0); - l = l + 1; + idat += vec4(1.0); + l += 1; continue; } _177.out_data[ident] = idat; diff --git a/reference/shaders/comp/return.comp b/reference/shaders/comp/return.comp index d23454c9..b3e51cc0 100644 --- a/reference/shaders/comp/return.comp +++ b/reference/shaders/comp/return.comp @@ -21,7 +21,7 @@ void main() return; } } - for (int i = 0; i < 20; i = i + 1) + for (int i = 0; i < 20; i += 1) { if (i == 10) { diff --git a/reference/shaders/comp/torture-loop.comp b/reference/shaders/comp/torture-loop.comp index 23afa073..083a3b5a 100644 --- a/reference/shaders/comp/torture-loop.comp +++ b/reference/shaders/comp/torture-loop.comp @@ -24,8 +24,8 @@ void main() k = _40; if (_40 < 10) { - idat = idat * 2.0; - k = k + 1; + idat *= 2.0; + k += 1; continue; } else @@ -33,16 +33,16 @@ void main() break; } } - for (uint i = 0u; i < 16u; i = i + uint(1), k = k + 1) + for (uint i = 0u; i < 16u; i += uint(1), k += 1) { - for (uint j = 0u; j < 30u; j = j + uint(1)) + for (uint j = 0u; j < 30u; j += uint(1)) { idat = _24.mvp * idat; } } do { - k = k + 1; + k += 1; } while (k > 10); _89.out_data[ident] = idat; } diff --git a/reference/shaders/desktop-only/comp/fp64.desktop.comp b/reference/shaders/desktop-only/comp/fp64.desktop.comp index b63fcdf7..18869eda 100644 --- a/reference/shaders/desktop-only/comp/fp64.desktop.comp +++ b/reference/shaders/desktop-only/comp/fp64.desktop.comp @@ -37,8 +37,8 @@ layout(binding = 3, std140) buffer SSBO3 void main() { - ssbo_0.a = ssbo_0.a + dvec4(10.0lf, 20.0lf, 30.0lf, 40.0lf); - ssbo_0.a = ssbo_0.a + dvec4(20.0lf); + ssbo_0.a += dvec4(10.0lf, 20.0lf, 30.0lf, 40.0lf); + ssbo_0.a += dvec4(20.0lf); dvec4 a = ssbo_0.a; dmat4 amat = ssbo_0.b; ssbo_0.a = abs(a); @@ -77,8 +77,8 @@ void main() k = lessThanEqual(a, a); k = greaterThan(a, a); k = greaterThanEqual(a, a); - ssbo_1.b.x = ssbo_1.b.x + 1.0lf; - ssbo_2.b[0].x = ssbo_2.b[0].x + 1.0lf; - ssbo_3.b[0].x = ssbo_3.b[0].x + 1.0lf; + ssbo_1.b.x += 1.0lf; + ssbo_2.b[0].x += 1.0lf; + ssbo_3.b[0].x += 1.0lf; } diff --git a/reference/shaders/desktop-only/comp/int64.desktop.comp b/reference/shaders/desktop-only/comp/int64.desktop.comp index b979cd2d..702456b3 100644 --- a/reference/shaders/desktop-only/comp/int64.desktop.comp +++ b/reference/shaders/desktop-only/comp/int64.desktop.comp @@ -36,17 +36,17 @@ layout(binding = 3, std140) buffer SSBO3 void main() { - ssbo_0.a = ssbo_0.a + i64vec4(10l, 20l, 30l, 40l); - ssbo_1.b = ssbo_1.b + u64vec4(999999999999999999ul, 8888888888888888ul, 77777777777777777ul, 6666666666666666ul); - ssbo_0.a = ssbo_0.a + i64vec4(20l); + ssbo_0.a += i64vec4(10l, 20l, 30l, 40l); + ssbo_1.b += u64vec4(999999999999999999ul, 8888888888888888ul, 77777777777777777ul, 6666666666666666ul); + ssbo_0.a += i64vec4(20l); ssbo_0.a = abs(ssbo_0.a + i64vec4(ssbo_1.b)); - ssbo_0.a = ssbo_0.a + i64vec4(1l); - ssbo_1.b = ssbo_1.b + u64vec4(i64vec4(1l)); - ssbo_0.a = ssbo_0.a - i64vec4(1l); - ssbo_1.b = ssbo_1.b - u64vec4(i64vec4(1l)); + ssbo_0.a += i64vec4(1l); + ssbo_1.b += u64vec4(i64vec4(1l)); + ssbo_0.a -= i64vec4(1l); + ssbo_1.b -= u64vec4(i64vec4(1l)); ssbo_1.b = doubleBitsToUint64(int64BitsToDouble(ssbo_0.a)); ssbo_0.a = doubleBitsToInt64(uint64BitsToDouble(ssbo_1.b)); - ssbo_2.a[0] = ssbo_2.a[0] + 1l; - ssbo_3.a[0] = ssbo_3.a[0] + 2l; + ssbo_2.a[0] += 1l; + ssbo_3.a[0] += 2l; } diff --git a/reference/shaders/frag/for-loop-init.frag b/reference/shaders/frag/for-loop-init.frag index 81161076..80fb3ac1 100644 --- a/reference/shaders/frag/for-loop-init.frag +++ b/reference/shaders/frag/for-loop-init.frag @@ -7,46 +7,46 @@ layout(location = 0) out mediump int FragColor; void main() { FragColor = 15; - for (mediump int i = 0; i < 25; i = i + 1) + for (mediump int i = 0; i < 25; i += 1) { - FragColor = FragColor + 10; + FragColor += 10; } - for (mediump int j = 4, i_1 = 1; i_1 < 30; i_1 = i_1 + 1, j = j + 4) + for (mediump int j = 4, i_1 = 1; i_1 < 30; i_1 += 1, j += 4) { - FragColor = FragColor + 11; + FragColor += 11; } mediump int k = 0; - for (; k < 20; k = k + 1) + for (; k < 20; k += 1) { - FragColor = FragColor + 12; + FragColor += 12; } - k = k + 3; - FragColor = FragColor + k; + k += 3; + FragColor += k; mediump int l; if (k == 40) { l = 0; - for (; l < 40; l = l + 1) + for (; l < 40; l += 1) { - FragColor = FragColor + 13; + FragColor += 13; } return; } else { l = k; - FragColor = FragColor + l; + FragColor += l; } mediump ivec2 i_2 = ivec2(0); - for (; i_2.x < 10; i_2.x = i_2.x + 1) + for (; i_2.x < 10; i_2.x += 1) { - FragColor = FragColor + i_2.y; + FragColor += i_2.y; } mediump int o = k; - for (mediump int m = k; m < 40; m = m + 1) + for (mediump int m = k; m < 40; m += 1) { - FragColor = FragColor + m; + FragColor += m; } - FragColor = FragColor + o; + FragColor += o; } diff --git a/reference/shaders/frag/ground.frag b/reference/shaders/frag/ground.frag index d9b655ea..4b0ea829 100644 --- a/reference/shaders/frag/ground.frag +++ b/reference/shaders/frag/ground.frag @@ -51,7 +51,7 @@ void main() vec3 base = mix(grass, snow, vec3(grass_snow)); float edge = smoothstep(0.699999988079071044921875, 0.75, Normal.y); Color = mix(dirt, base, vec3(edge)); - Color = Color * Color; + Color *= Color; float Roughness = 1.0 - (edge * grass_snow); highp vec3 param_1 = Color; highp vec3 param_2 = Normal; diff --git a/reference/shaders/tese/water_tess.tese b/reference/shaders/tese/water_tess.tese index 029f5c9e..758c2b4e 100644 --- a/reference/shaders/tese/water_tess.tese +++ b/reference/shaders/tese/water_tess.tese @@ -46,7 +46,7 @@ void main() vec2 param_1 = tess_coord; mediump vec2 lod = lod_factor(param_1); vec2 tex = pos * _31.uInvHeightmapSize; - pos = pos * _31.uScale.xy; + pos *= _31.uScale.xy; mediump float delta_mod = exp2(lod.x); vec2 off = _31.uInvHeightmapSize * delta_mod; vGradNormalTex = vec4(tex + (_31.uInvHeightmapSize * 0.5), tex * _31.uScale.zw); @@ -54,7 +54,7 @@ void main() vec2 param_3 = off; vec2 param_4 = lod; vec3 height_displacement = sample_height_displacement(param_2, param_3, param_4); - pos = pos + height_displacement.yz; + pos += height_displacement.yz; vWorld = vec3(pos.x, height_displacement.x, pos.y); gl_Position = _31.uMVP * vec4(vWorld, 1.0); } diff --git a/reference/shaders/vert/ground.vert b/reference/shaders/vert/ground.vert index 41de1063..44b19a99 100644 --- a/reference/shaders/vert/ground.vert +++ b/reference/shaders/vert/ground.vert @@ -101,8 +101,8 @@ void main() vec2 Offset = _381.InvGroundSize_PatchScale.xy * exp2(lod.x); float Elevation = mix(textureLod(TexHeightmap, NormalizedPos + (Offset * 0.5), lod.x).x, textureLod(TexHeightmap, NormalizedPos + (Offset * 1.0), lod.x + 1.0).x, lod.y); vec3 WorldPos = vec3(NormalizedPos.x, Elevation, NormalizedPos.y); - WorldPos = WorldPos * _381.GroundScale.xyz; - WorldPos = WorldPos + _381.GroundPosition.xyz; + WorldPos *= _381.GroundScale.xyz; + WorldPos += _381.GroundPosition.xyz; EyeVec = WorldPos - _58.g_CamPos.xyz; TexCoord = NormalizedPos + (_381.InvGroundSize_PatchScale.xy * 0.5); gl_Position = (((_58.g_ViewProj_Row0 * WorldPos.x) + (_58.g_ViewProj_Row1 * WorldPos.y)) + (_58.g_ViewProj_Row2 * WorldPos.z)) + _58.g_ViewProj_Row3; diff --git a/reference/shaders/vert/ocean.vert b/reference/shaders/vert/ocean.vert index 76d07675..c542fe25 100644 --- a/reference/shaders/vert/ocean.vert +++ b/reference/shaders/vert/ocean.vert @@ -124,8 +124,8 @@ void main() vec2 Offset = (_405.InvOceanSize_PatchScale.xy * exp2(lod.x)) * _405.NormalTexCoordScale.zw; vec3 Displacement = mix(textureLod(TexDisplacement, NormalizedTex + (Offset * 0.5), lod.x).yxz, textureLod(TexDisplacement, NormalizedTex + (Offset * 1.0), lod.x + 1.0).yxz, vec3(lod.y)); vec3 WorldPos = vec3(NormalizedPos.x, 0.0, NormalizedPos.y) + Displacement; - WorldPos = WorldPos * _405.OceanScale.xyz; - WorldPos = WorldPos + _405.OceanPosition.xyz; + WorldPos *= _405.OceanScale.xyz; + WorldPos += _405.OceanPosition.xyz; EyeVec = WorldPos - _58.g_CamPos.xyz; TexCoord = vec4(NormalizedTex, NormalizedTex * _405.NormalTexCoordScale.xy) + ((_405.InvOceanSize_PatchScale.xyxy * 0.5) * _405.NormalTexCoordScale.zwzw); gl_Position = (((_58.g_ViewProj_Row0 * WorldPos.x) + (_58.g_ViewProj_Row1 * WorldPos.y)) + (_58.g_ViewProj_Row2 * WorldPos.z)) + _58.g_ViewProj_Row3; diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index e669078b..78fbc3ab 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -3426,6 +3426,23 @@ bool CompilerGLSL::skip_argument(uint32_t id) const return false; } +bool CompilerGLSL::optimize_read_modify_write(const string &lhs, const string &rhs) +{ + // Do this with strings because we have a very clear pattern we can check for and it avoids + // adding lots of special cases to the code emission. + auto index = rhs.find(lhs); + if (index != 0) + return false; + + // TODO: Shift operators, but it's not important for now. + auto op = rhs.find_first_of("+-/*%|&^", lhs.size() + 1); + if (op != lhs.size() + 1) + return false; + + statement(lhs, " ", rhs[op], "=", rhs.substr(lhs.size() + 2), ";"); + return true; +} + void CompilerGLSL::emit_instruction(const Instruction &instruction) { auto ops = stream(instruction); @@ -3503,7 +3520,12 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction) // For this case, we don't need to invalidate anything and emit any opcode. if (lhs != rhs) { - statement(lhs, " = ", rhs, ";"); + // Tries to optimize assignments like " = op expr". + // While this is purely cosmetic, this is important for legacy ESSL where loop + // variable increments must be in either i++ or i += const-expr. + // Without this, we end up with i = i + 1, which is correct GLSL, but not correct GLES 2.0. + if (!optimize_read_modify_write(lhs, rhs)) + statement(lhs, " = ", rhs, ";"); register_write(ops[0]); } } diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index d6e5a177..a8824f6b 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -390,6 +390,8 @@ protected: void find_static_extensions(); std::string emit_for_loop_initializers(const SPIRBlock &block); + + bool optimize_read_modify_write(const std::string &lhs, const std::string &rhs); }; } From d11b8aa3ef68c8c247e3154c957d94626395a60c Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 16 Dec 2016 13:24:49 +0100 Subject: [PATCH 5/7] Optimize += 1, -= 1 to ++, --. Purely cosmetic, but easier to read. --- reference/shaders/comp/cfg.comp | 2 +- reference/shaders/comp/dowhile.comp | 2 +- reference/shaders/comp/loop.comp | 18 +++++++++--------- reference/shaders/comp/return.comp | 2 +- reference/shaders/comp/torture-loop.comp | 8 ++++---- reference/shaders/frag/for-loop-init.frag | 12 ++++++------ shaders/frag/for-loop-init.frag | 2 +- spirv_glsl.cpp | 12 +++++++++++- 8 files changed, 34 insertions(+), 24 deletions(-) diff --git a/reference/shaders/comp/cfg.comp b/reference/shaders/comp/cfg.comp index ed4a45c8..707968e7 100644 --- a/reference/shaders/comp/cfg.comp +++ b/reference/shaders/comp/cfg.comp @@ -64,7 +64,7 @@ void test() } } float h; - for (int i = 0; i < 20; i += 1, h += 10.0) + for (int i = 0; i < 20; i++, h += 10.0) { } _11.data = h; diff --git a/reference/shaders/comp/dowhile.comp b/reference/shaders/comp/dowhile.comp index dabf08f5..16ba4001 100644 --- a/reference/shaders/comp/dowhile.comp +++ b/reference/shaders/comp/dowhile.comp @@ -22,7 +22,7 @@ void main() do { idat = _28.mvp * idat; - i += 1; + i++; } while (i < 16); _52.out_data[ident] = idat; } diff --git a/reference/shaders/comp/loop.comp b/reference/shaders/comp/loop.comp index fbc7d4a0..9853acaa 100644 --- a/reference/shaders/comp/loop.comp +++ b/reference/shaders/comp/loop.comp @@ -23,7 +23,7 @@ void main() do { k *= 2; - i += uint(1); + i++; } while (i < ident); } switch (k) @@ -32,7 +32,7 @@ void main() { for (;;) { - i += uint(1); + i++; if (i > 10u) { break; @@ -58,11 +58,11 @@ void main() while (k < 10) { idat *= 2.0; - k += 1; + k++; } - for (uint i_1 = 0u; i_1 < 16u; i_1 += uint(1), k += 1) + for (uint i_1 = 0u; i_1 < 16u; i_1++, k++) { - for (uint j = 0u; j < 30u; j += uint(1)) + for (uint j = 0u; j < 30u; j++) { idat = _24.mvp * idat; } @@ -70,7 +70,7 @@ void main() k = 0; for (;;) { - k += 1; + k++; if (k > 10) { k += 2; @@ -86,18 +86,18 @@ void main() k = 0; do { - k += 1; + k++; } while (k > 10); int l = 0; for (;;) { if (l == 5) { - l += 1; + l++; continue; } idat += vec4(1.0); - l += 1; + l++; continue; } _177.out_data[ident] = idat; diff --git a/reference/shaders/comp/return.comp b/reference/shaders/comp/return.comp index b3e51cc0..20d61d25 100644 --- a/reference/shaders/comp/return.comp +++ b/reference/shaders/comp/return.comp @@ -21,7 +21,7 @@ void main() return; } } - for (int i = 0; i < 20; i += 1) + for (int i = 0; i < 20; i++) { if (i == 10) { diff --git a/reference/shaders/comp/torture-loop.comp b/reference/shaders/comp/torture-loop.comp index 083a3b5a..ae183190 100644 --- a/reference/shaders/comp/torture-loop.comp +++ b/reference/shaders/comp/torture-loop.comp @@ -25,7 +25,7 @@ void main() if (_40 < 10) { idat *= 2.0; - k += 1; + k++; continue; } else @@ -33,16 +33,16 @@ void main() break; } } - for (uint i = 0u; i < 16u; i += uint(1), k += 1) + for (uint i = 0u; i < 16u; i++, k++) { - for (uint j = 0u; j < 30u; j += uint(1)) + for (uint j = 0u; j < 30u; j++) { idat = _24.mvp * idat; } } do { - k += 1; + k++; } while (k > 10); _89.out_data[ident] = idat; } diff --git a/reference/shaders/frag/for-loop-init.frag b/reference/shaders/frag/for-loop-init.frag index 80fb3ac1..95483d9f 100644 --- a/reference/shaders/frag/for-loop-init.frag +++ b/reference/shaders/frag/for-loop-init.frag @@ -7,16 +7,16 @@ layout(location = 0) out mediump int FragColor; void main() { FragColor = 15; - for (mediump int i = 0; i < 25; i += 1) + for (mediump int i = 0; i < 25; i++) { FragColor += 10; } - for (mediump int j = 4, i_1 = 1; i_1 < 30; i_1 += 1, j += 4) + for (mediump int j = 4, i_1 = 1; i_1 < 30; i_1++, j += 4) { FragColor += 11; } mediump int k = 0; - for (; k < 20; k += 1) + for (; k < 20; k++) { FragColor += 12; } @@ -26,7 +26,7 @@ void main() if (k == 40) { l = 0; - for (; l < 40; l += 1) + for (; l < 40; l++) { FragColor += 13; } @@ -38,12 +38,12 @@ void main() FragColor += l; } mediump ivec2 i_2 = ivec2(0); - for (; i_2.x < 10; i_2.x += 1) + for (; i_2.x < 10; i_2.x += 4) { FragColor += i_2.y; } mediump int o = k; - for (mediump int m = k; m < 40; m += 1) + for (mediump int m = k; m < 40; m++) { FragColor += m; } diff --git a/shaders/frag/for-loop-init.frag b/shaders/frag/for-loop-init.frag index 78b8903e..af2af7e2 100644 --- a/shaders/frag/for-loop-init.frag +++ b/shaders/frag/for-loop-init.frag @@ -37,7 +37,7 @@ void main() } // Vectors cannot be loop variables - for (ivec2 i = ivec2(0); i.x < 10; i.x++) + for (ivec2 i = ivec2(0); i.x < 10; i.x += 4) { FragColor += i.y; } diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 78fbc3ab..360c4152 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -3430,6 +3430,9 @@ bool CompilerGLSL::optimize_read_modify_write(const string &lhs, const string &r { // Do this with strings because we have a very clear pattern we can check for and it avoids // adding lots of special cases to the code emission. + if (rhs.size() < lhs.size() + 3) + return false; + auto index = rhs.find(lhs); if (index != 0) return false; @@ -3439,7 +3442,14 @@ bool CompilerGLSL::optimize_read_modify_write(const string &lhs, const string &r if (op != lhs.size() + 1) return false; - statement(lhs, " ", rhs[op], "=", rhs.substr(lhs.size() + 2), ";"); + char bop = rhs[op]; + auto expr = rhs.substr(lhs.size() + 3); + // Try to find increments and decrements. Makes it look neater as += 1, -= 1 is fairly rare to see in real code. + // Find some common patterns which are equivalent. + if ((bop == '+' || bop == '-') && (expr == "1" || expr == "uint(1)" || expr == "1u" || expr == "int(1u)")) + statement(lhs, bop, bop, ";"); + else + statement(lhs, " ", bop, "= ", expr, ";"); return true; } From 45c797d54cc7c15325857a9bd31c0938abdb8014 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 16 Dec 2016 13:48:30 +0100 Subject: [PATCH 6/7] Improve debuggability of Travis CI when things go wrong. --- .travis.yml | 2 +- reference/shaders/frag/for-loop-init.frag | 2 +- shaders/frag/for-loop-init.frag | 2 +- test_shaders.py | 8 ++++++++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 84bb1357..593d472b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,5 +22,5 @@ script: - cd glslang && cmake . && make -j2 && cd .. - cd SPIRV-Tools && cmake . && make -j2 && cd .. - make -j2 - - PATH=$PATH:./glslang/StandAlone:./SPIRV-Tools/tools + - PATH=./glslang/StandAlone:./SPIRV-Tools/tools:$PATH - ./test_shaders.py shaders diff --git a/reference/shaders/frag/for-loop-init.frag b/reference/shaders/frag/for-loop-init.frag index 95483d9f..98f1b8ce 100644 --- a/reference/shaders/frag/for-loop-init.frag +++ b/reference/shaders/frag/for-loop-init.frag @@ -6,7 +6,7 @@ layout(location = 0) out mediump int FragColor; void main() { - FragColor = 15; + FragColor = 16; for (mediump int i = 0; i < 25; i++) { FragColor += 10; diff --git a/shaders/frag/for-loop-init.frag b/shaders/frag/for-loop-init.frag index af2af7e2..0cde2676 100644 --- a/shaders/frag/for-loop-init.frag +++ b/shaders/frag/for-loop-init.frag @@ -4,7 +4,7 @@ layout(location = 0) out int FragColor; void main() { - FragColor = 15; + FragColor = 16; // Basic loop variable. for (int i = 0; i < 25; i++) diff --git a/test_shaders.py b/test_shaders.py index 1e51aa4b..daac82a6 100755 --- a/test_shaders.py +++ b/test_shaders.py @@ -137,6 +137,14 @@ def regression_check(shader, glsl, update, keep): shutil.move(glsl, reference) else: print('Generated GLSL in {} does not match reference {}!'.format(glsl, reference)) + with open(glsl, 'r') as f: + print('') + print('Generated:') + print('======================') + print(f.read()) + print('======================') + print('') + # Otherwise, fail the test. Keep the shader file around so we can inspect. if not keep: os.remove(glsl) From 44b3216611df7449a7b4f5f22a2f91a587fcb447 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Fri, 16 Dec 2016 14:01:09 +0100 Subject: [PATCH 7/7] Sort loop variables to make sure Travis CI runs are reproducable. --- reference/shaders/frag/for-loop-init.frag | 2 +- spirv_cross.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/reference/shaders/frag/for-loop-init.frag b/reference/shaders/frag/for-loop-init.frag index 98f1b8ce..7c22e5c7 100644 --- a/reference/shaders/frag/for-loop-init.frag +++ b/reference/shaders/frag/for-loop-init.frag @@ -11,7 +11,7 @@ void main() { FragColor += 10; } - for (mediump int j = 4, i_1 = 1; i_1 < 30; i_1++, j += 4) + for (mediump int i_1 = 1, j = 4; i_1 < 30; i_1++, j += 4) { FragColor += 11; } diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 39ea21f6..64130575 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -3025,6 +3025,9 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry) // We have a loop variable. header_block.loop_variables.push_back(loop_variable.first); + // Need to sort here as variables come from an unordered container, and pushing stuff in wrong order + // will break reproducability in regression runs. + sort(begin(header_block.loop_variables), end(header_block.loop_variables)); get(loop_variable.first).loop_variable = true; } }