[Merge] (ShaderTranspiler): land the single-implementation subroutine lowering and the imageSize select ladder

MobileGL/MG_Test/Program/ProgramUtilTest.cpp

@@ -4678,3 +4678,143 @@ void main() { clearInstance.b = 0u; }
     EXPECT_EQ(unqualified.count("Clear"), 1u)
         << "the unambiguous block alongside it is still recognised";
 }
+
+// KHR-GL43.shader_image_size.advanced-nonMS-* is nothing but its passing twin basic-nonMS-* plus a
+// GLSL subroutine, and glslang refuses the keyword outright when the target is SPIR-V ("subroutine
+// : not allowed when generating SPIR-V"), so every stage of those shaders failed to compile. The
+// lowering turns a subroutine uniform with exactly ONE compatible subroutine - the case where GL
+// 4.3 core 7.9 makes a direct call indistinguishable from a dispatch, because every legal value of
+// the uniform selects that one function - into a forwarding call.
+TEST_F(ProgramUtilTest, PreprocessLowersSingleImplementationSubroutineToAForwardingCall) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    String source = R"(#version 430 core
+layout(binding = 0, rgba32i) writeonly uniform iimage2D g_result;
+subroutine void FuncType(int coord);
+subroutine uniform FuncType g_func;
+void main() {
+  int coord = gl_VertexID;
+  g_func(coord);
+}
+subroutine(FuncType) void Func0(int coord) {
+  imageStore(g_result, ivec2(coord, 0), ivec4(imageSize(g_result), 0, 0));
+}
+)";
+    const SizeT mainLine = std::count(source.begin(), source.begin() + source.find("void main"), '\n');
+
+    PreprocessShaderSource(ShaderStage::Vertex, source);
+
+    EXPECT_EQ(source.find("subroutine"), String::npos) << "the keyword glslang refuses must be gone";
+    EXPECT_NE(source.find("void g_func(int mgl_sr_arg0);"), String::npos)
+        << "the subroutine uniform becomes a prototype under its own name, so call sites stand";
+    EXPECT_NE(source.find("g_func(coord);"), String::npos) << "the call site is untouched";
+    EXPECT_NE(source.find("void Func0(int coord)"), String::npos)
+        << "the compatible subroutine keeps its body and only sheds the qualifier";
+    EXPECT_NE(source.find("Func0(mgl_sr_arg0);"), String::npos) << "the forwarding body";
+    // The forwarding body has to come after every definition it names: the CTS shaders define
+    // their subroutine BELOW the function that calls through the uniform.
+    EXPECT_LT(source.find("void Func0(int coord)"), source.find("Func0(mgl_sr_arg0);"));
+    // Blanking preserves newlines, and the prototype is single-line, so glslang's diagnostics still
+    // point at the line the application wrote.
+    EXPECT_EQ(std::count(source.begin(), source.begin() + source.find("void main"), '\n'), mainLine)
+        << "the rewrite must not move a single line";
+
+    ShaderAttrib attrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = source};
+    auto res = ShaderCompiler::CompileShader(attrib);
+    if (!res) {
+        FAIL() << "errc: " << res.error().errc << "\nlog: " << res.error().log << "\nsource:\n" << source;
+    }
+}
+
+// The forwarding function is rebuilt from the subroutine TYPE declaration, so it has to carry the
+// parameter qualifiers and array shapes across (an  parameter that arrives by value writes
+// nothing back) and has to return the forwarded value for a non-void subroutine.
+TEST_F(ProgramUtilTest, PreprocessSubroutineForwardingKeepsParameterQualifiersAndReturnsValues) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    String source = R"(#version 430 core
+subroutine float Blend(const int k, out vec4 rgba, float weights[2]);
+subroutine uniform Blend g_blend;
+out vec4 fragColor;
+void main() {
+  vec4 rgba;
+  float w[2] = float[2](0.25, 0.75);
+  fragColor = rgba * g_blend(1, rgba, w);
+}
+subroutine(Blend) float Mix(const int k, out vec4 rgba, float weights[2]) {
+  rgba = vec4(weights[0], weights[1], float(k), 1.0);
+  return weights[0];
+}
+)";
+
+    PreprocessShaderSource(ShaderStage::Fragment, source);
+
+    EXPECT_NE(source.find("float g_blend(const int mgl_sr_arg0, out vec4 mgl_sr_arg1, float mgl_sr_arg2 [ 2 ]);"),
+              String::npos)
+        << "qualifiers and the array declarator have to survive, under generated names";
+    EXPECT_NE(source.find("return Mix(mgl_sr_arg0, mgl_sr_arg1, mgl_sr_arg2);"), String::npos)
+        << "a non-void subroutine has to have its value forwarded back";
+
+    ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source};
+    auto res = ShaderCompiler::CompileShader(attrib);
+    if (!res) {
+        FAIL() << "errc: " << res.error().errc << "\nlog: " << res.error().log << "\nsource:\n" << source;
+    }
+}
+
+// Two compatible subroutines is genuine dynamic selection, which MobileGL does not implement:
+// glUniformSubroutinesuiv is still a stub and nothing reflects the subroutine interfaces. Pinning
+// such a shader to one of the alternatives would render silently wrong, so the whole rewrite is
+// abandoned and the source is left exactly as it arrived.
+TEST_F(ProgramUtilTest, PreprocessLeavesMultiImplementationSubroutinesAlone) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    String source = R"(#version 430 core
+subroutine void FuncType(int coord);
+subroutine uniform FuncType g_func;
+out vec4 fragColor;
+void main() {
+  g_func(1);
+  fragColor = vec4(1.0);
+}
+subroutine(FuncType) void Func0(int coord) { fragColor = vec4(float(coord)); }
+subroutine(FuncType) void Func1(int coord) { fragColor = vec4(float(coord) * 2.0); }
+)";
+    const String before = source;
+
+    PreprocessShaderSource(ShaderStage::Fragment, source);
+
+    EXPECT_EQ(source, before) << "an unimplementable dispatch must not be quietly pinned to one arm";
+}
+
+// An ARRAY of subroutine uniforms indexes the dispatch at the call site ("g_func[i](x)"), which is
+// the same dynamic selection - and a subroutine declared inside a #if arm cannot be reasoned about
+// at all, because the forwarding bodies this appends are unconditional.
+TEST_F(ProgramUtilTest, PreprocessLeavesArrayAndConditionalSubroutinesAlone) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    String arrayed = R"(#version 430 core
+subroutine void FuncType(int coord);
+subroutine uniform FuncType g_func[2];
+out vec4 fragColor;
+void main() { g_func[0](1); fragColor = vec4(1.0); }
+subroutine(FuncType) void Func0(int coord) { fragColor = vec4(float(coord)); }
+)";
+    const String arrayedBefore = arrayed;
+    PreprocessShaderSource(ShaderStage::Fragment, arrayed);
+    EXPECT_EQ(arrayed, arrayedBefore) << "an arrayed subroutine uniform is a dispatch, not a call";
+
+    String conditional = R"(#version 430 core
+out vec4 fragColor;
+#ifdef USE_SUBROUTINE
+subroutine void FuncType(int coord);
+subroutine uniform FuncType g_func;
+#endif
+void main() { fragColor = vec4(1.0); }
+subroutine(FuncType) void Func0(int coord) { fragColor = vec4(float(coord)); }
+)";
+    const String conditionalBefore = conditional;
+    PreprocessShaderSource(ShaderStage::Fragment, conditional);
+    EXPECT_EQ(conditional, conditionalBefore)
+        << "an inactive #if arm must not have an unconditional forwarding body appended for it";
+}

MobileGL/MG_Test/ShaderTranspiler/LegalizeResourceArrayIndexTest.cpp

@@ -342,6 +342,21 @@ void main() {
     // An imageAtomic* reaches the array through OpImageTexelPointer, and running one per element
     // would perform every other element's atomic as well. The pass has to decline rather than
     // lower this.
+    // imageSize() on a dynamically indexed image array. The query carries the image in the same
+    // leading operand position as an imageLoad and answers with an int vector, so the select
+    // ladder spells it exactly - and unlike a read it touches no memory at all, so evaluating it
+    // for every element cannot even return undefined data.
+    constexpr const char* kUniformIndexedImageSizeQuery = R"(#version 450 core
+layout(local_size_x = 1) in;
+layout(rgba32f, binding = 0) uniform image2D g_image[4];
+layout(rgba32f, binding = 4) uniform image2DArray g_layered[2];
+layout(std430, binding = 8) buffer Out { ivec2 size; int layers; } g_out;
+uniform int g_index;
+void main() {
+    g_out.size = imageSize(g_image[g_index]);
+    g_out.layers = imageSize(g_layered[g_index]).z;
+}
+)";
     constexpr const char* kUniformIndexedImageAtomic = R"(#version 450 core
 layout(local_size_x = 1) in;
 layout(r32ui, binding = 0) uniform uimage2D g_image[4];
@@ -514,6 +529,36 @@ TEST(LegalizeResourceArrayIndexPass, LeavesADynamicallyIndexedSamplerArrayByteId
     EXPECT_EQ(output, input);
 }
 
+// imageSize() on a dynamically indexed image array used to lose the whole stage: the consumer
+// whitelist accepted only OpImageRead/OpImageWrite, so the chain was declined and the illegal
+// subscript reached the ES compiler intact. It is the same select ladder as a read - the query
+// takes the image in in-operand 0 and produces an int vector - and it reads no memory, so the
+// elements the shader did not ask for cost nothing but the instruction.
+TEST(LegalizeResourceArrayIndexPass, LowersAUniformIndexedImageSizeQueryToSelects) {
+    const Vector<Uint32> input = CompileCompute(kUniformIndexedImageSizeQuery);
+    ASSERT_FALSE(input.empty());
+    EXPECT_TRUE(HasDynamicImageArrayIndex(input));
+    EXPECT_EQ(CountOpcode(input, spv::Op::OpImageQuerySize), 2u);
+    EXPECT_EQ(CountOpcode(input, spv::Op::OpSelect), 0u);
+
+    Vector<Uint32> output;
+    ASSERT_TRUE(ShaderCompiler::LegalizeResourceArrayIndexingForEssl(input, output, true));
+    ASSERT_FALSE(output.empty());
+    EXPECT_FALSE(HasDynamicImageArrayIndex(output));
+    // Four elements for g_image and two for g_layered, one query apiece, and one select per
+    // element past the first of each ladder.
+    EXPECT_EQ(CountOpcode(output, spv::Op::OpImageQuerySize), 6u);
+    EXPECT_EQ(CountOpcode(output, spv::Op::OpSelect), 4u);
+    EXPECT_EQ(CountOpcode(output, spv::Op::OpSwitch), 0u) << "a query produces a value, so no control flow";
+    EXPECT_TRUE(Validates(output));
+
+    const EsslAttempt after = EmitEssl(output);
+    ASSERT_TRUE(after.succeeded) << after.error;
+    for (int element = 0; element < 4; ++element) {
+        EXPECT_NE(after.text.find("g_image[" + std::to_string(element) + "]"), String::npos) << after.text;
+    }
+}
+
 // An imageAtomic* is the shape the lowering must refuse: its per-element rebuild would run every
 // other element's read-modify-write. Declining leaves the illegal subscript in place - which is
 // what the latched warning in LegalizeResourceArrayIndexingForEssl is for - but a half-transform

MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp

@@ -930,6 +930,405 @@ namespace {
         }
     }
 
+    // Index one past the token that closes the group tokens[open] opens, or the token count when
+    // the group is never closed. Nesting of the SAME bracket pair is counted, everything else is
+    // skipped, so a '(' inside a '[' run cannot confuse a bracket walk and vice versa.
+    SizeT FindGroupEnd(const Vector<CodeToken>& tokens, SizeT open, char opener, char closer) {
+        int depth = 0;
+        for (SizeT i = open; i < tokens.size(); ++i) {
+            if (tokens[i].text.size() != 1) continue;
+            if (tokens[i].text[0] == opener) {
+                ++depth;
+            } else if (tokens[i].text[0] == closer && --depth == 0) {
+                return i + 1;
+            }
+        }
+        return tokens.size();
+    }
+
+    // Token text joined by single spaces. Token text is comment-free by construction (the
+    // tokenizer reads a masked source), so this is how a rewritten declaration is rebuilt without
+    // dragging a comment - or a newline - into a line the rewrite promises to keep single-line.
+    String JoinTokenText(const Vector<CodeToken>& tokens, SizeT begin, SizeT end) {
+        String text;
+        for (SizeT i = begin; i < end; ++i) {
+            if (!text.empty()) text += ' ';
+            text += tokens[i].text;
+        }
+        return text;
+    }
+
+    // Erase a span for the compiler while keeping every later offset - and every LINE NUMBER -
+    // exactly where it was, so edits collected against one token scan all stay valid and glslang's
+    // diagnostics still point at the line the application wrote.
+    void BlankSpan(MobileGL::String& source, SizeT begin, SizeT end) {
+        for (SizeT i = begin; i < end && i < source.size(); ++i) {
+            if (source[i] != '\n' && source[i] != '\r') {
+                source[i] = ' ';
+            }
+        }
+    }
+
+    bool IsParameterQualifierKeyword(const String& text) {
+        static constexpr std::string_view kQualifiers[] = {
+            "const",    "in",       "out",      "inout",     "highp",     "mediump", "lowp",
+            "precise",  "coherent", "volatile", "restrict",  "readonly",  "writeonly",
+        };
+        return std::find(std::begin(kQualifiers), std::end(kQualifiers), std::string_view(text)) !=
+            std::end(kQualifiers);
+    }
+
+    // The #if/#ifdef/#ifndef nesting in effect at each offset, as (offset, depth) marks. Every
+    // mark takes effect at the END of the directive line that changed the depth.
+    Vector<std::pair<SizeT, int>> BuildConditionalDepthMarks(const MobileGL::String& source,
+                                                            const Vector<std::pair<SizeT, SizeT>>& ranges) {
+        Vector<std::pair<SizeT, int>> marks;
+        marks.emplace_back(static_cast<SizeT>(0), 0);
+        int depth = 0;
+        for (const std::pair<SizeT, SizeT>& range : ranges) {
+            SizeT pos = range.first;
+            SkipDirectiveWhitespace(source, pos, range.second);
+            if (pos >= range.second || source[pos] != '#') continue;
+            ++pos;
+            SkipDirectiveWhitespace(source, pos, range.second);
+            const String name = ReadDirectiveIdentifier(source, pos, range.second);
+            if (name == "if" || name == "ifdef" || name == "ifndef") {
+                ++depth;
+            } else if (name == "endif") {
+                if (depth > 0) --depth;
+            } else {
+                continue;
+            }
+            marks.emplace_back(range.second, depth);
+        }
+        return marks;
+    }
+
+    int ConditionalDepthAt(const Vector<std::pair<SizeT, int>>& marks, SizeT offset) {
+        const auto next = std::upper_bound(marks.begin(), marks.end(), offset,
+                                           [](SizeT value, const std::pair<SizeT, int>& mark) {
+                                               return value < mark.first;
+                                           });
+        return next == marks.begin() ? 0 : std::prev(next)->second;
+    }
+
+    struct SubroutineParameters {
+        Vector<String> declarations; // "in highp float mgl_sr_arg0", ready for a parameter list
+        Vector<String> arguments;    // "mgl_sr_arg0", ready for a forwarding call
+    };
+
+    // One parameter of a subroutine TYPE declaration, whose name (if it even has one) this rewrite
+    // replaces with a generated one. The shape read is
+    //     <qualifier>* <typeName> <arrayOfType>? <name>? <arrayOfName>?
+    // which is the whole of the GLSL parameter grammar; anything that does not fit is refused so
+    // the caller can abandon the rewrite rather than emit a guess.
+    bool AppendSubroutineParameter(const Vector<CodeToken>& tokens, SizeT begin, SizeT end, SizeT index,
+                                   SubroutineParameters& parameters) {
+        if (begin >= end) return false;
+
+        SizeT cursor = begin;
+        while (cursor < end && IsParameterQualifierKeyword(tokens[cursor].text)) {
+            ++cursor;
+        }
+        if (cursor >= end || !IsIdentifierToken(tokens[cursor])) return false;
+        ++cursor;
+        while (cursor < end && tokens[cursor].text == "[") { // "float[4] a"
+            const SizeT close = FindGroupEnd(tokens, cursor, '[', ']');
+            if (close > end) return false;
+            cursor = close;
+        }
+        const String typeText = JoinTokenText(tokens, begin, cursor);
+
+        String arraySuffix;
+        if (cursor < end) { // the declared parameter name, which the generated one replaces
+            if (!IsIdentifierToken(tokens[cursor])) return false;
+            const SizeT afterName = cursor + 1;
+            cursor = afterName;
+            while (cursor < end && tokens[cursor].text == "[") { // "float a[4]"
+                const SizeT close = FindGroupEnd(tokens, cursor, '[', ']');
+                if (close > end) return false;
+                cursor = close;
+            }
+            if (cursor != end) return false;
+            arraySuffix = JoinTokenText(tokens, afterName, end);
+        }
+
+        const String name = "mgl_sr_arg" + std::to_string(index);
+        parameters.declarations.push_back(typeText + " " + name + (arraySuffix.empty() ? "" : " " + arraySuffix));
+        parameters.arguments.push_back(name);
+        return true;
+    }
+
+    // The parameter list between (but not including) the parentheses of a subroutine type
+    // declaration. "()" and "(void)" are both the empty list.
+    bool ParseSubroutineParameters(const Vector<CodeToken>& tokens, SizeT begin, SizeT end,
+                                   SubroutineParameters& parameters) {
+        if (begin >= end) return true;
+        if (end == begin + 1 && tokens[begin].text == "void") return true;
+
+        SizeT parameterBegin = begin;
+        SizeT index = 0;
+        for (SizeT i = begin; i <= end; ++i) {
+            if (i < end) {
+                if (tokens[i].text == "[") { // a comma inside a subscript is not a separator
+                    const SizeT close = FindGroupEnd(tokens, i, '[', ']');
+                    if (close > end) return false;
+                    i = close - 1;
+                    continue;
+                }
+                if (tokens[i].text != ",") continue;
+            }
+            if (!AppendSubroutineParameter(tokens, parameterBegin, i, index, parameters)) return false;
+            ++index;
+            parameterBegin = i + 1;
+        }
+        return true;
+    }
+
+    // GLSL subroutines (ARB_shader_subroutine, core since 4.00).
+    //
+    // glslang refuses the keyword outright once the target is SPIR-V - "'subroutine' : not allowed
+    // when generating SPIR-V", "feature not yet implemented" - so a shader that declares one never
+    // produces a module at all and the whole program is lost at COMPILE time. That is the entire
+    // failure of KHR-GL43.shader_image_size.advanced-nonMS-*: its subroutine-free twin
+    // basic-nonMS-* drives the identical image battery through the identical imageSize() calls on
+    // the identical targets and passes on every stage.
+    //
+    // The rewrite is confined to the case where it is provably a no-op on semantics: a subroutine
+    // uniform whose type has EXACTLY ONE compatible subroutine. GL 4.3 core 7.9 leaves the value of
+    // a subroutine uniform implementation-dependent until glUniformSubroutinesuiv sets it, so with
+    // a single compatible subroutine every legal value of that uniform selects the same function
+    // and a direct call is indistinguishable from a dispatch under any GL state. A type with two or
+    // more compatible subroutines genuinely needs the dynamic selection MobileGL does not implement
+    // (glUniformSubroutinesuiv is still a stub, and nothing reflects the subroutine interfaces), so
+    // it is left to fail at compile time exactly as it does today rather than silently pinned to
+    // one of the alternatives.
+    //
+    //     subroutine void FuncType(int coord);          ->  (blanked)
+    //     subroutine uniform FuncType g_func;           ->  void g_func(int mgl_sr_arg0);
+    //     subroutine(FuncType) void Func0(int c) { }    ->  void Func0(int c) { }
+    //                                                       ...plus, appended at end of source,
+    //                                                       void g_func(int mgl_sr_arg0) {
+    //                                                           Func0(mgl_sr_arg0);
+    //                                                       }
+    //
+    // Naming the forwarding function after the subroutine UNIFORM is what leaves every CALL site
+    // untouched - "g_func(coord)" already reads as a call - and that name is free precisely because
+    // the declaration that held it is gone. The forwarding body has to be appended rather than
+    // written in place because the compatible subroutine is routinely defined AFTER the function
+    // that calls through the uniform (the CTS shaders define theirs below main()); at end of source
+    // every definition it names is already in scope, and a prototype at the old declaration site
+    // keeps the call sites legal.
+    //
+    // All-or-nothing, in the discipline of the scanners below it: an array subroutine uniform, a
+    // subroutine token inside a #if arm or a macro body, an unbalanced file, a type that is never
+    // declared - anything outside the grammar abandons the whole pass with the source untouched,
+    // which is exactly today's behaviour.
+    void LowerShaderSubroutines(MobileGL::String& source) {
+        if (source.find("subroutine") == MobileGL::String::npos) return;
+
+        const Vector<CodeToken> tokens = TokenizeCode(source);
+        const SizeT count = tokens.size();
+        if (count < 4 || !HasBalancedBraces(tokens)) return;
+
+        const Vector<std::pair<SizeT, SizeT>> directiveRanges = FindDirectiveLineRanges(source);
+        const Vector<std::pair<SizeT, int>> conditionalDepth =
+            BuildConditionalDepthMarks(source, directiveRanges);
+
+        struct SubroutineType {
+            String returnText; // empty until the type declaration itself is seen
+            SubroutineParameters parameters;
+            Vector<String> implementations; // compatible subroutines, in declaration order
+        };
+        struct UniformSite {
+            SizeT begin = 0; // first byte of the declaration, layout(...) qualifier included
+            SizeT end = 0;   // one past its ';'
+            String typeName;
+            Vector<String> variables;
+        };
+        struct BlankEdit {
+            SizeT begin;
+            SizeT end;
+        };
+
+        MobileGL::UnorderedMap<String, SubroutineType> types;
+        Vector<UniformSite> uniformSites;
+        Vector<BlankEdit> blanks;
+
+        SizeT braceDepth = 0;
+        for (SizeT i = 0; i < count; ++i) {
+            const CodeToken& token = tokens[i];
+            if (token.text.size() == 1) {
+                if (token.text[0] == '{') {
+                    ++braceDepth;
+                    continue;
+                }
+                if (token.text[0] == '}') {
+                    if (braceDepth > 0) --braceDepth;
+                    continue;
+                }
+            }
+            if (token.text != "subroutine") continue;
+
+            // Nothing here may reason about a subroutine that is not unconditionally at file
+            // scope: the forwarding bodies this appends are unconditional, so a declaration that
+            // only exists in one #if arm (or inside a macro body) would have them naming a
+            // function that is not there.
+            if (braceDepth != 0 || IsInDirectiveLine(directiveRanges, token.begin) ||
+                ConditionalDepthAt(conditionalDepth, token.begin) != 0) {
+                return;
+            }
+            if (i + 1 >= count) return;
+
+            // (a) `[layout(...)] subroutine uniform <TypeName> <var>[, <var>]... ;`
+            if (tokens[i + 1].text == "uniform") {
+                UniformSite site;
+                site.begin = token.begin;
+                if (i >= 2 && tokens[i - 1].text == ")") {
+                    SizeT open = i - 1;
+                    int depth = 1;
+                    while (depth > 0) {
+                        if (open == 0) return;
+                        --open;
+                        if (tokens[open].text == ")") {
+                            ++depth;
+                        } else if (tokens[open].text == "(") {
+                            --depth;
+                        }
+                    }
+                    if (open == 0 || tokens[open - 1].text != "layout") return;
+                    site.begin = tokens[open - 1].begin;
+                }
+
+                SizeT cursor = i + 2;
+                if (cursor >= count || !IsIdentifierToken(tokens[cursor])) return;
+                site.typeName = tokens[cursor].text;
+                ++cursor;
+                while (true) {
+                    if (cursor >= count || !IsIdentifierToken(tokens[cursor])) return;
+                    site.variables.push_back(tokens[cursor].text);
+                    ++cursor;
+                    if (cursor >= count) return;
+                    if (tokens[cursor].text == ",") {
+                        ++cursor;
+                        continue;
+                    }
+                    // An ARRAY of subroutine uniforms indexes the dispatch itself
+                    // ("g_func[i](x)"), which is the dynamic selection this rewrite refuses.
+                    if (tokens[cursor].text != ";") return;
+                    break;
+                }
+                site.end = tokens[cursor].end;
+                uniformSites.push_back(std::move(site));
+                i = cursor;
+                continue;
+            }
+
+            // (b) `subroutine(<TypeName>, ...) <ret> <name>(<params>) { ... }` - a definition,
+            // which only has to shed the qualifier to become an ordinary function.
+            if (tokens[i + 1].text == "(") {
+                const SizeT listEnd = FindGroupEnd(tokens, i + 1, '(', ')');
+                if (listEnd >= count) return;
+                Vector<String> listed;
+                for (SizeT t = i + 2; t + 1 < listEnd; ++t) {
+                    if (tokens[t].text == ",") continue;
+                    if (!IsIdentifierToken(tokens[t])) return;
+                    listed.push_back(tokens[t].text);
+                }
+                if (listed.empty()) return;
+
+                SizeT paren = listEnd;
+                while (paren < count && tokens[paren].text != "(") {
+                    const String& text = tokens[paren].text;
+                    if (text == "{" || text == "}" || text == ";" || text == ",") return;
+                    ++paren;
+                }
+                if (paren >= count || paren == listEnd || !IsIdentifierToken(tokens[paren - 1])) return;
+
+                for (const String& typeName : listed) {
+                    types[typeName].implementations.push_back(tokens[paren - 1].text);
+                }
+                blanks.push_back({token.begin, tokens[listEnd - 1].end});
+                i = listEnd - 1;
+                continue;
+            }
+
+            // (c) `subroutine <ret> <TypeName>(<params>);` - the type declaration.
+            SizeT paren = i + 1;
+            while (paren < count && tokens[paren].text != "(") {
+                const String& text = tokens[paren].text;
+                if (text == "{" || text == "}" || text == ";" || text == ",") return;
+                ++paren;
+            }
+            if (paren >= count || paren == i + 1 || !IsIdentifierToken(tokens[paren - 1])) return;
+            const SizeT listEnd = FindGroupEnd(tokens, paren, '(', ')');
+            if (listEnd >= count || tokens[listEnd].text != ";") return;
+
+            SubroutineType& type = types[tokens[paren - 1].text];
+            if (!type.returnText.empty()) return; // declared twice; out of scope
+            type.returnText = JoinTokenText(tokens, i + 1, paren - 1);
+            if (type.returnText.empty()) return;
+            if (!ParseSubroutineParameters(tokens, paren + 1, listEnd - 1, type.parameters)) return;
+            blanks.push_back({token.begin, tokens[listEnd].end});
+            i = listEnd;
+        }
+
+        if (blanks.empty() && uniformSites.empty()) return;
+
+        for (const UniformSite& site : uniformSites) {
+            const auto known = types.find(site.typeName);
+            if (known == types.end() || known->second.returnText.empty()) return;
+            if (known->second.implementations.size() != 1) return;
+        }
+
+        String appended;
+        Vector<std::pair<SizeT, String>> prototypes; // (offset, text), applied back to front
+        for (const UniformSite& site : uniformSites) {
+            const SubroutineType& type = types.at(site.typeName);
+            String parameterList;
+            for (const String& declaration : type.parameters.declarations) {
+                if (!parameterList.empty()) parameterList += ", ";
+                parameterList += declaration;
+            }
+            String arguments;
+            for (const String& argument : type.parameters.arguments) {
+                if (!arguments.empty()) arguments += ", ";
+                arguments += argument;
+            }
+
+            String text;
+            for (const String& variable : site.variables) {
+                const String signature = type.returnText + " " + variable + "(" + parameterList + ")";
+                text += signature + "; ";
+                appended += signature + " {\n    " + (type.returnText == "void" ? "" : "return ") +
+                    type.implementations.front() + "(" + arguments + ");\n}\n";
+            }
+            prototypes.emplace_back(site.begin, std::move(text));
+        }
+
+        // Blanking first keeps every collected offset valid (it preserves length AND newlines), so
+        // only the prototype insertions - which are single-line, and so cost no line numbers - have
+        // to run back to front.
+        for (const BlankEdit& blank : blanks) {
+            BlankSpan(source, blank.begin, blank.end);
+        }
+        for (const UniformSite& site : uniformSites) {
+            BlankSpan(source, site.begin, site.end);
+        }
+        std::sort(prototypes.begin(), prototypes.end(),
+                  [](const std::pair<SizeT, String>& a, const std::pair<SizeT, String>& b) {
+                      return a.first < b.first;
+                  });
+        for (auto it = prototypes.rbegin(); it != prototypes.rend(); ++it) {
+            source.insert(it->first, it->second);
+        }
+
+        if (!appended.empty()) {
+            if (!source.empty() && source.back() != '\n') source += '\n';
+            source += appended;
+        }
+    }
+
     // Rewrite the `packed` / `shared` block-packing qualifiers inside layout(...) declarations to
     // `std140`. Desktop GL leaves the memory layout of such blocks to the implementation and the
     // app must query member offsets; MobileGL's SPIR-V pipeline always lays uniform blocks out as
@@ -1080,6 +1479,10 @@ namespace MobileGL {
                 // declaration identical.
                 SizeNonFinalUnsizedBufferBlockMembers(source);
 
+                // Before the builtin-shadowing rename, so the forwarding functions this synthesizes
+                // are just as visible to it as the ones the application wrote.
+                LowerShaderSubroutines(source);
+
                 RenameBuiltinShadowingFunctions(source);
 
                 ModernizeLegacyGLSL(stage, source, afterVersion);

MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.cpp

@@ -737,12 +737,18 @@ namespace MobileGL {
                         return;
                     case spv::Op::OpImageWrite:
                     case spv::Op::OpImageRead:
+                    // imageSize()/imageSamples() carry the image in the same leading operand
+                    // position as an OpImageRead and produce an int or int vector, so the same
+                    // select ladder rebuilds them exactly - and a size query touches no memory
+                    // at all, which makes evaluating it for every element strictly safer than
+                    // the read the ladder was written for.
+                    case spv::Op::OpImageQuerySize:
+                    case spv::Op::OpImageQuerySizeLod:
                         if (consumer == nullptr) consumer = user;
                         return;
                     default:
-                        // A sampled-image construction, a query, a copy, an argument to a
+                        // A sampled-image construction, a copy, an argument to a function:
-                        // function: shapes whose per-element rebuild this pass cannot spell
+                        // shapes whose per-element rebuild this pass cannot spell exactly.
-                        // exactly.
                         unsupportedConsumer = true;
                         return;
                     }
@@ -762,7 +768,7 @@ namespace MobileGL {
 
                 return consumer->opcode() == spv::Op::OpImageWrite
                            ? LowerImageWrite(accessChain, arrayLength, load, consumer)
-                           : LowerImageRead(accessChain, arrayLength, load, consumer);
+                           : LowerImageReadOrQuery(accessChain, arrayLength, load, consumer);
             }
 
             // Drops |load| and |accessChain| once the rewrite above has taken their last user,
@@ -865,38 +871,42 @@ namespace MobileGL {
                 return LoweringOutcome::Changed;
             }
 
-            // A read needs no control flow: read every element through a constant index and pick
+            // A value-producing consumer - an OpImageRead, or an imageSize()/imageSamples() query -
+            // needs no control flow: run it against every element through a constant index and pick
             // with OpSelect. The selection happens on the RESULT, not on the image object - an
             // opaque type may not be selected at all (pre-1.4 OpSelect takes pointers, scalars
             // and vectors only, and ESSL has no ternary on an image), so what is duplicated is
-            // the OpImageRead.
+            // the consuming instruction itself. Every such consumer carries the image in in-operand
+            // 0 and nothing else that is per-element, so one rebuild spells all of them.
             //
-            // Reading the elements the shader did not ask for is safe: every one of them is an
+            // Running the elements the shader did not ask for is safe: every one of them is an
             // image this stage already declares, and GL 4.6 7.11.2 makes a load through an
             // image unit whose binding is missing or incompatible return undefined DATA - never
-            // an error, and never a fault - which the select then discards. Contrast an
+            // an error, and never a fault - which the select then discards. A size query does not
-            // imageAtomic*, which LowerImageChain refuses for exactly the opposite reason.
+            // even touch memory. Contrast an imageAtomic*, which LowerImageChain refuses for
+            // exactly the opposite reason.
             LegalizeResourceArrayIndexPass::LoweringOutcome
-            LegalizeResourceArrayIndexPass::LowerImageRead(Instruction* accessChain, uint32_t arrayLength,
+            LegalizeResourceArrayIndexPass::LowerImageReadOrQuery(Instruction* accessChain,
-                                                           Instruction* load, Instruction* imageRead) {
+                                                                  uint32_t arrayLength, Instruction* load,
+                                                                  Instruction* consumer) {
                 auto* irContext = context();
                 uint32_t conditionTypeId = 0;
                 uint32_t dimension = 0;
-                if (!TryGetSelectConditionType(irContext, imageRead->type_id(), &conditionTypeId,
+                if (!TryGetSelectConditionType(irContext, consumer->type_id(), &conditionTypeId,
                                                &dimension)) {
-                    MGLOG_D("[spirv] image array index: read type is not selectable, declining");
+                    MGLOG_D("[spirv] image array index: result type is not selectable, declining");
                     return LoweringOutcome::Declined;
                 }
                 const uint32_t boolTypeId = irContext->get_type_mgr()->GetBoolTypeId();
                 const uint32_t indexId = accessChain->GetSingleWordInOperand(1);
                 const uint32_t imageTypeId = load->type_id();
                 std::vector<Operand> tailOperands;
-                for (uint32_t i = 1; i < imageRead->NumInOperands(); ++i) {
+                for (uint32_t i = 1; i < consumer->NumInOperands(); ++i) {
-                    tailOperands.push_back(imageRead->GetInOperand(i));
+                    tailOperands.push_back(consumer->GetInOperand(i));
                 }
 
                 InstructionBuilder builder(
-                    irContext, imageRead,
+                    irContext, consumer,
                     IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
 
                 uint32_t selectedId = 0;
@@ -906,18 +916,18 @@ namespace MobileGL {
                         CloneChainWithConstantIndex(builder, irContext, accessChain, constantId);
                     Instruction* elementImage = builder.AddLoad(imageTypeId, elementChain->result_id());
 
-                    std::vector<Operand> readOperands;
+                    std::vector<Operand> elementOperands;
-                    readOperands.push_back({SPV_OPERAND_TYPE_ID, {elementImage->result_id()}});
+                    elementOperands.push_back({SPV_OPERAND_TYPE_ID, {elementImage->result_id()}});
                     for (const Operand& tailOperand : tailOperands) {
-                        readOperands.push_back(tailOperand);
+                        elementOperands.push_back(tailOperand);
                     }
-                    Instruction* elementRead = builder.AddInstruction(
+                    Instruction* elementResult = builder.AddInstruction(
-                        MakeUnique<Instruction>(irContext, spv::Op::OpImageRead, imageRead->type_id(),
+                        MakeUnique<Instruction>(irContext, consumer->opcode(), consumer->type_id(),
-                                                irContext->TakeNextId(), readOperands));
+                                                irContext->TakeNextId(), elementOperands));
                     if (element == 0) {
                         // Element 0 is the else-arm of the whole ladder, so an out-of-range
                         // index reads it - an undefined element for an undefined index.
-                        selectedId = elementRead->result_id();
+                        selectedId = elementResult->result_id();
                         continue;
                     }
 
@@ -929,17 +939,17 @@ namespace MobileGL {
                         conditionId = builder.AddCompositeConstruct(conditionTypeId, components)->result_id();
                     }
                     selectedId = builder
-                                     .AddSelect(imageRead->type_id(), conditionId,
+                                     .AddSelect(consumer->type_id(), conditionId,
-                                                elementRead->result_id(), selectedId)
+                                                elementResult->result_id(), selectedId)
                                      ->result_id();
                 }
 
-                irContext->ReplaceAllUsesWith(imageRead->result_id(), selectedId);
+                irContext->ReplaceAllUsesWith(consumer->result_id(), selectedId);
-                irContext->KillInst(imageRead);
+                irContext->KillInst(consumer);
                 KillImageChainIfDead(accessChain, load);
                 irContext->InvalidateAnalysesExceptFor(IRContext::kAnalysisNone);
-                MGLOG_D("[spirv] image array index: lowered a dynamic imageLoad to %u constant-indexed "
+                MGLOG_D("[spirv] image array index: lowered a dynamic image read/query to %u "
-                        "reads",
+                        "constant-indexed operations",
                         arrayLength);
                 return LoweringOutcome::Changed;
             }

MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.h

@@ -146,9 +146,10 @@ namespace MobileGL {
                 LoweringOutcome LowerImageWrite(spvtools::opt::Instruction* accessChain, uint32_t arrayLength,
                                                 spvtools::opt::Instruction* load,
                                                 spvtools::opt::Instruction* imageWrite);
-                LoweringOutcome LowerImageRead(spvtools::opt::Instruction* accessChain, uint32_t arrayLength,
+                LoweringOutcome LowerImageReadOrQuery(spvtools::opt::Instruction* accessChain,
-                                               spvtools::opt::Instruction* load,
+                                                      uint32_t arrayLength,
-                                               spvtools::opt::Instruction* imageRead);
+                                                      spvtools::opt::Instruction* load,
+                                                      spvtools::opt::Instruction* consumer);
 
                 Mode m_mode;
             };