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/.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/comp/cfg.comp b/reference/shaders/comp/cfg.comp index 63d95ab1..707968e7 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++, h += 10.0) { } _11.data = h; diff --git a/reference/shaders/comp/dowhile.comp b/reference/shaders/comp/dowhile.comp index 01449340..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 = i + 1; + i++; } 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 a9848dda..9853acaa 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++; } while (i < ident); } switch (k) @@ -32,7 +32,7 @@ void main() { for (;;) { - i = i + uint(1); + i++; if (i > 10u) { break; @@ -45,7 +45,7 @@ void main() { for (;;) { - i = i + 2u; + i += 2u; if (i > 20u) { break; @@ -57,14 +57,12 @@ void main() } while (k < 10) { - idat = idat * 2.0; - k = k + 1; + idat *= 2.0; + k++; } - 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++, k++) { - uint j = 0u; - for (; j < 30u; j = j + uint(1)) + for (uint j = 0u; j < 30u; j++) { idat = _24.mvp * idat; } @@ -72,34 +70,34 @@ void main() k = 0; for (;;) { - k = k + 1; + k++; 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++; } while (k > 10); int l = 0; for (;;) { if (l == 5) { - l = l + 1; + l++; continue; } - idat = idat + vec4(1.0); - l = l + 1; + idat += vec4(1.0); + l++; continue; } _177.out_data[ident] = idat; diff --git a/reference/shaders/comp/return.comp b/reference/shaders/comp/return.comp index ebbaac2d..20d61d25 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++) { if (i == 10) { diff --git a/reference/shaders/comp/torture-loop.comp b/reference/shaders/comp/torture-loop.comp index abcbb7c5..ae183190 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++; continue; } else @@ -33,18 +33,16 @@ void main() break; } } - uint i = 0u; - for (; i < 16u; i = i + uint(1), k = k + 1) + for (uint i = 0u; i < 16u; i++, k++) { - uint j = 0u; - for (; j < 30u; j = j + uint(1)) + for (uint j = 0u; j < 30u; j++) { idat = _24.mvp * idat; } } do { - k = k + 1; + k++; } 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 new file mode 100644 index 00000000..7c22e5c7 --- /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 = 16; + for (mediump int i = 0; i < 25; i++) + { + FragColor += 10; + } + for (mediump int i_1 = 1, j = 4; i_1 < 30; i_1++, j += 4) + { + FragColor += 11; + } + mediump int k = 0; + for (; k < 20; k++) + { + FragColor += 12; + } + k += 3; + FragColor += k; + mediump int l; + if (k == 40) + { + l = 0; + for (; l < 40; l++) + { + FragColor += 13; + } + return; + } + else + { + l = k; + FragColor += l; + } + mediump ivec2 i_2 = ivec2(0); + 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++) + { + FragColor += m; + } + 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/shaders/frag/for-loop-init.frag b/shaders/frag/for-loop-init.frag new file mode 100644 index 00000000..0cde2676 --- /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 = 16; + + // 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 += 4) + { + 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_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..64130575 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -2909,15 +2909,33 @@ 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) { 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. + // 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. + 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 +2947,87 @@ 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); + // 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; + } } diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index dca6796a..dcddfc90 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) { @@ -3426,6 +3428,33 @@ 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. + if (rhs.size() < lhs.size() + 3) + return false; + + 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; + + 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; +} + void CompilerGLSL::emit_instruction(const Instruction &instruction) { auto ops = stream(instruction); @@ -3492,6 +3521,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]); @@ -3501,7 +3532,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]); } } @@ -4922,7 +4958,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; } @@ -5347,6 +5385,47 @@ 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); + + // 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; + } for (auto &v : func.local_variables) { @@ -5367,24 +5446,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; @@ -5614,6 +5688,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)); @@ -5635,8 +5739,8 @@ 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), - ")"); + statement("for (", emit_for_loop_initializers(block), "; ", to_expression(block.condition), "; ", + emit_continue_block(block.continue_block), ")"); break; case SPIRBlock::WhileLoop: @@ -5682,8 +5786,8 @@ 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), - ")"); + statement("for (", emit_for_loop_initializers(block), "; ", to_expression(child.condition), "; ", + emit_continue_block(block.continue_block), ")"); break; case SPIRBlock::WhileLoop: @@ -5727,6 +5831,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) @@ -5740,15 +5845,19 @@ 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)) { 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 @@ -5757,7 +5866,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) { @@ -5785,6 +5897,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..a8824f6b 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -388,6 +388,10 @@ 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); + + bool optimize_read_modify_write(const std::string &lhs, const std::string &rhs); }; } 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)