Reland "Runtime effects: Detect passthrough sample calls automatically"

As explained, this is *very* conservative. It only works when the child
is sampled from within main, and using a direct reference to the coords
parameter. If that parameter is ever modified (even after being used),
the optimization doesn't happen. For most cases, this is fine.

Reland changes the logic in GrSkSLFP slightly, to avoid turning all
samples into pass-through when a child is sampled with both pass-through
and explicit coordinates.

Bug: skia:11869
Change-Id: Iec18f059b4e78df0d2f53449aa0c2945c58a58f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401677
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>

src/core/SkRuntimeEffect.cpp

@@ -159,7 +159,19 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl,
     settings.fForceNoInline = options.forceNoInline;
     settings.fAllowNarrowingConversions = true;
 
-    const SkSL::FunctionDefinition* main = nullptr;
+    // Find 'main', then locate the sample coords parameter. (It might not be present.)
+    const SkSL::FunctionDefinition* main = SkSL::Program_GetFunction(*program, "main");
+    if (!main) {
+        RETURN_FAILURE("missing 'main' function");
+    }
+    const auto& mainParams = main->declaration().parameters();
+    auto iter = std::find_if(mainParams.begin(), mainParams.end(), [](const SkSL::Variable* p) {
+        return p->modifiers().fLayout.fBuiltin == SK_MAIN_COORDS_BUILTIN;
+    });
+    const SkSL::ProgramUsage::VariableCounts sampleCoordsUsage =
+            iter != mainParams.end() ? program->usage()->get(**iter)
+                                     : SkSL::ProgramUsage::VariableCounts{};
+
     uint32_t flags = 0;
     switch (kind) {
         case SkSL::ProgramKind::kRuntimeColorFilter: flags |= kAllowColorFilter_Flag; break;
@@ -168,7 +180,9 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl,
                                                                kAllowShader_Flag);    break;
         default: SkUNREACHABLE;
     }
-    if (SkSL::Analysis::ReferencesSampleCoords(*program)) {
+
+
+    if (sampleCoordsUsage.fRead || sampleCoordsUsage.fWrite) {
         flags |= kUsesSampleCoords_Flag;
     }
 
@@ -180,7 +194,7 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl,
     // this can be simpler. There is no way for color filters to refer to sk_FragCoord or sample
     // coords in that mode.
     if ((flags & kAllowColorFilter_Flag) &&
-        (SkSL::Analysis::ReferencesFragCoords(*program) || (flags & kUsesSampleCoords_Flag))) {
+        ((flags & kUsesSampleCoords_Flag) || SkSL::Analysis::ReferencesFragCoords(*program))) {
         flags &= ~kAllowColorFilter_Flag;
     }
 
@@ -203,7 +217,8 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl,
             // Child effects that can be sampled ('shader' or 'colorFilter')
             if (varType.isEffectChild()) {
                 children.push_back(var.name());
-                sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(*program, var));
+                sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(
+                        *program, var, sampleCoordsUsage.fWrite != 0));
             }
             // 'uniform' variables
             else if (var.modifiers().fFlags & SkSL::Modifiers::kUniform_Flag) {
@@ -234,18 +249,6 @@ SkRuntimeEffect::Result SkRuntimeEffect::Make(SkString sksl,
                 uniforms.push_back(uni);
             }
         }
-        // Functions
-        else if (elem->is<SkSL::FunctionDefinition>()) {
-            const auto& func = elem->as<SkSL::FunctionDefinition>();
-            const SkSL::FunctionDeclaration& decl = func.declaration();
-            if (decl.isMain()) {
-                main = &func;
-            }
-        }
-    }
-
-    if (!main) {
-        RETURN_FAILURE("missing 'main' function");
     }
 
 #undef RETURN_FAILURE

src/gpu/effects/GrSkSLFP.cpp

@@ -96,6 +96,22 @@ public:
             }
 
             String sampleChild(int index, String coords) override {
+                // If the child was sampled using the coords passed to main (and they are never
+                // modified), then we will have marked the child as PassThrough. The code generator
+                // doesn't know that, and still supplies coords. Inside invokeChild, we assert that
+                // any coords passed for a PassThrough child match args.fSampleCoords exactly.
+                //
+                // Normally, this is valid. Here, we *copied* the sample coords to a local variable
+                // (so that they're mutable in the runtime effect SkSL). Thus, the coords string we
+                // get here is the name of the local copy, and fSampleCoords still points to the
+                // unmodified original (which might be a varying, for example).
+                // To prevent the assert, we pass the empty string in this case. Note that for
+                // children sampled like this, invokeChild doesn't even use the coords parameter,
+                // except for that assert.
+                const GrFragmentProcessor* child = fArgs.fFp.childProcessor(index);
+                if (child && !child->isSampledWithExplicitCoords()) {
+                    coords.clear();
+                }
                 return String(fSelf->invokeChild(index, fInputColor, fArgs, coords).c_str());
             }
 

src/sksl/SkSLAnalysis.cpp

@@ -74,8 +74,8 @@ static bool is_sample_call_to_fp(const FunctionCall& fc, const Variable& fp) {
 // Visitor that determines the merged SampleUsage for a given child 'fp' in the program.
 class MergeSampleUsageVisitor : public ProgramVisitor {
 public:
-    MergeSampleUsageVisitor(const Context& context, const Variable& fp)
+    MergeSampleUsageVisitor(const Context& context, const Variable& fp, bool writesToSampleCoords)
-            : fContext(context), fFP(fp) {}
+            : fContext(context), fFP(fp), fWritesToSampleCoords(writesToSampleCoords) {}
 
     SampleUsage visit(const Program& program) {
         fUsage = SampleUsage(); // reset to none
@@ -86,6 +86,7 @@ public:
 protected:
     const Context& fContext;
     const Variable& fFP;
+    const bool fWritesToSampleCoords;
     SampleUsage fUsage;
 
     bool visitExpression(const Expression& e) override {
@@ -97,7 +98,18 @@ protected:
                 if (fc.arguments().size() >= 2) {
                     const Expression* coords = fc.arguments()[1].get();
                     if (coords->type() == *fContext.fTypes.fFloat2) {
-                        fUsage.merge(SampleUsage::Explicit());
+                        // If the coords are a direct reference to the program's sample-coords,
+                        // and those coords are never modified, we can conservatively turn this
+                        // into PassThrough sampling. In all other cases, we consider it Explicit.
+                        if (!fWritesToSampleCoords && coords->is<VariableReference>() &&
+                            coords->as<VariableReference>()
+                                            .variable()
+                                            ->modifiers()
+                                            .fLayout.fBuiltin == SK_MAIN_COORDS_BUILTIN) {
+                            fUsage.merge(SampleUsage::PassThrough());
+                        } else {
+                            fUsage.merge(SampleUsage::Explicit());
+                        }
                     } else if (coords->type() == *fContext.fTypes.fFloat3x3) {
                         // Determine the type of matrix for this call site
                         if (coords->isConstantOrUniform()) {
@@ -570,8 +582,10 @@ public:
 ////////////////////////////////////////////////////////////////////////////////
 // Analysis
 
-SampleUsage Analysis::GetSampleUsage(const Program& program, const Variable& fp) {
+SampleUsage Analysis::GetSampleUsage(const Program& program,
-    MergeSampleUsageVisitor visitor(*program.fContext, fp);
+                                     const Variable& fp,
+                                     bool writesToSampleCoords) {
+    MergeSampleUsageVisitor visitor(*program.fContext, fp, writesToSampleCoords);
     return visitor.visit(program);
 }
 

src/sksl/SkSLAnalysis.h

@@ -33,7 +33,14 @@ enum class VariableRefKind : int8_t;
  * Provides utilities for analyzing SkSL statically before it's composed into a full program.
  */
 struct Analysis {
-    static SampleUsage GetSampleUsage(const Program& program, const Variable& fp);
+    /**
+     * Determines how `program` samples `fp`. By default, assumes that the sample coords
+     * (SK_MAIN_COORDS_BUILTIN) might be modified, so `sample(fp, sampleCoords)` is treated as
+     * Explicit. If writesToSampleCoords is false, treats that as PassThrough, instead.
+     */
+    static SampleUsage GetSampleUsage(const Program& program,
+                                      const Variable& fp,
+                                      bool writesToSampleCoords = true);
 
     static bool ReferencesBuiltin(const Program& program, int builtin);
 

src/sksl/ir/SkSLProgram.h

@@ -199,6 +199,8 @@ struct Program {
         return result;
     }
 
+    const ProgramUsage* usage() const { return fUsage.get(); }
+
     std::unique_ptr<String> fSource;
     std::unique_ptr<ProgramConfig> fConfig;
     std::shared_ptr<Context> fContext;

tests/SkRuntimeEffectTest.cpp

@@ -16,6 +16,7 @@
 #include "src/core/SkColorSpacePriv.h"
 #include "src/core/SkTLazy.h"
 #include "src/gpu/GrColor.h"
+#include "src/gpu/GrFragmentProcessor.h"
 #include "tests/Test.h"
 
 #include <algorithm>
@@ -525,3 +526,51 @@ DEF_TEST(SkRuntimeColorFilterFlags, r) {
         REPORTER_ASSERT(r, filter && !filter->isAlphaUnchanged());
     }
 }
+
+DEF_TEST(SkRuntimeShaderSampleUsage, r) {
+    auto test = [&](const char* src, bool expectExplicit) {
+        auto [effect, err] =
+                SkRuntimeEffect::MakeForShader(SkStringPrintf("uniform shader child; %s", src));
+        REPORTER_ASSERT(r, effect);
+
+        auto child = GrFragmentProcessor::MakeColor({ 1, 1, 1, 1 });
+        auto fp = effect->makeFP(nullptr, &child, 1);
+        REPORTER_ASSERT(r, fp);
+
+        REPORTER_ASSERT(r, fp->childProcessor(0)->isSampledWithExplicitCoords() == expectExplicit);
+    };
+
+    // This test verifies that we detect calls to sample where the coords are the same as those
+    // passed to main. In those cases, it's safe to turn the "explicit" sampling into "passthrough"
+    // sampling. This optimization is implemented very conservatively.
+
+    // Cases where our optimization is valid, and works:
+
+    // Direct use of passed-in coords
+    test("half4 main(float2 xy) { return sample(child, xy); }", false);
+    // Sample with passed-in coords, read (but don't write) sample coords elsewhere
+    test("half4 main(float2 xy) { return sample(child, xy) + sin(xy.x); }", false);
+
+    // Cases where our optimization is not valid, and does not happen:
+
+    // Sampling with values completely unrelated to passed-in coords
+    test("half4 main(float2 xy) { return sample(child, float2(0, 0)); }", true);
+    // Use of expression involving passed in coords
+    test("half4 main(float2 xy) { return sample(child, xy * 0.5); }", true);
+    // Use of coords after modification
+    test("half4 main(float2 xy) { xy *= 2; return sample(child, xy); }", true);
+    // Use of coords after modification via out-param call
+    test("void adjust(inout float2 xy) { xy *= 2; }"
+         "half4 main(float2 xy) { adjust(xy); return sample(child, xy); }", true);
+
+    // There should (must) not be any false-positive cases. There are false-negatives.
+    // In all of these cases, our optimization would be valid, but does not happen:
+
+    // Direct use of passed-in coords, modified after use
+    test("half4 main(float2 xy) { half4 c = sample(child, xy); xy *= 2; return c; }", true);
+    // Passed-in coords copied to a temp variable
+    test("half4 main(float2 xy) { float2 p = xy; return sample(child, p); }", true);
+    // Use of coords passed to helper function
+    test("half4 helper(float2 xy) { return sample(child, xy); }"
+         "half4 main(float2 xy) { return helper(xy); }", true);
+}