diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 05c4bff8..388320d2 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/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"; +} diff --git a/MobileGL/MG_Test/ShaderTranspiler/LegalizeResourceArrayIndexTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/LegalizeResourceArrayIndexTest.cpp index 1c9cc7c3..27e02fd3 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/LegalizeResourceArrayIndexTest.cpp +++ b/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 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 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 diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index fc67c811..2c977494 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/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& 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& 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> BuildConditionalDepthMarks(const MobileGL::String& source, + const Vector>& ranges) { + Vector> marks; + marks.emplace_back(static_cast(0), 0); + int depth = 0; + for (const std::pair& 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>& marks, SizeT offset) { + const auto next = std::upper_bound(marks.begin(), marks.end(), offset, + [](SizeT value, const std::pair& mark) { + return value < mark.first; + }); + return next == marks.begin() ? 0 : std::prev(next)->second; + } + + struct SubroutineParameters { + Vector declarations; // "in highp float mgl_sr_arg0", ready for a parameter list + Vector 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 + // * ? ? ? + // 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& 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& 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 tokens = TokenizeCode(source); + const SizeT count = tokens.size(); + if (count < 4 || !HasBalancedBraces(tokens)) return; + + const Vector> directiveRanges = FindDirectiveLineRanges(source); + const Vector> conditionalDepth = + BuildConditionalDepthMarks(source, directiveRanges); + + struct SubroutineType { + String returnText; // empty until the type declaration itself is seen + SubroutineParameters parameters; + Vector 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 variables; + }; + struct BlankEdit { + SizeT begin; + SizeT end; + }; + + MobileGL::UnorderedMap types; + Vector uniformSites; + Vector 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 [, ]... ;` + 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(, ...) () { ... }` - 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 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 ();` - 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> 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& a, const std::pair& 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); diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.cpp index fb0d5fbb..c96c8da8 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.cpp +++ b/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 - // function: shapes whose per-element rebuild this pass cannot spell - // exactly. + // A sampled-image construction, a copy, an argument to a function: + // shapes whose per-element rebuild this pass cannot spell 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 - // imageAtomic*, which LowerImageChain refuses for exactly the opposite reason. + // an error, and never a fault - which the select then discards. A size query does not + // even touch memory. Contrast an imageAtomic*, which LowerImageChain refuses for + // exactly the opposite reason. LegalizeResourceArrayIndexPass::LoweringOutcome - LegalizeResourceArrayIndexPass::LowerImageRead(Instruction* accessChain, uint32_t arrayLength, - Instruction* load, Instruction* imageRead) { + LegalizeResourceArrayIndexPass::LowerImageReadOrQuery(Instruction* accessChain, + 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 tailOperands; - for (uint32_t i = 1; i < imageRead->NumInOperands(); ++i) { - tailOperands.push_back(imageRead->GetInOperand(i)); + for (uint32_t i = 1; i < consumer->NumInOperands(); ++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 readOperands; - readOperands.push_back({SPV_OPERAND_TYPE_ID, {elementImage->result_id()}}); + std::vector elementOperands; + 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( - MakeUnique(irContext, spv::Op::OpImageRead, imageRead->type_id(), - irContext->TakeNextId(), readOperands)); + Instruction* elementResult = builder.AddInstruction( + MakeUnique(irContext, consumer->opcode(), consumer->type_id(), + 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, - elementRead->result_id(), selectedId) + .AddSelect(consumer->type_id(), conditionId, + elementResult->result_id(), selectedId) ->result_id(); } - irContext->ReplaceAllUsesWith(imageRead->result_id(), selectedId); - irContext->KillInst(imageRead); + irContext->ReplaceAllUsesWith(consumer->result_id(), selectedId); + irContext->KillInst(consumer); KillImageChainIfDead(accessChain, load); irContext->InvalidateAnalysesExceptFor(IRContext::kAnalysisNone); - MGLOG_D("[spirv] image array index: lowered a dynamic imageLoad to %u constant-indexed " - "reads", + MGLOG_D("[spirv] image array index: lowered a dynamic image read/query to %u " + "constant-indexed operations", arrayLength); return LoweringOutcome::Changed; } diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.h index f77d542d..0eae085c 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/LegalizeResourceArrayIndexPass.h +++ b/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, - spvtools::opt::Instruction* load, - spvtools::opt::Instruction* imageRead); + LoweringOutcome LowerImageReadOrQuery(spvtools::opt::Instruction* accessChain, + uint32_t arrayLength, + spvtools::opt::Instruction* load, + spvtools::opt::Instruction* consumer); Mode m_mode; };