added support for interpreting generic SkSL functions

Bug: skia:
Change-Id: I575b91c654393cc6cfedea617852598e8595d5ed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209809
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

src/sksl/SkSLByteCode.h

@@ -71,6 +71,8 @@ enum class ByteCodeInstruction : uint8_t {
     kPushImmediate,
     kRemainderS,
     kRemainderU,
+    // Followed by a byte indicating the number of slots being returned
+    kReturn,
     kStore,
     kStoreGlobal,
     // Followed by a count byte (1-4), and then one byte per swizzle component (0-3). Expects the

src/sksl/SkSLByteCodeGenerator.cpp

@@ -690,8 +690,9 @@ void ByteCodeGenerator::writeIfStatement(const IfStatement& i) {
 }
 
 void ByteCodeGenerator::writeReturnStatement(const ReturnStatement& r) {
-    // not yet implemented
+    this->writeExpression(*r.fExpression);
-    abort();
+    this->write(ByteCodeInstruction::kReturn);
+    this->write8(r.fExpression->fType.columns() * r.fExpression->fType.rows());
 }
 
 void ByteCodeGenerator::writeSwitchStatement(const SwitchStatement& r) {

src/sksl/SkSLCompiler.cpp

@@ -57,12 +57,8 @@ static const char* SKSL_FP_INCLUDE =
 #include "sksl_fp.inc"
 ;
 
-static const char* SKSL_PIPELINE_STAGE_INCLUDE =
+static const char* SKSL_GENERIC_INCLUDE =
-#include "sksl_pipeline.inc"
+#include "sksl_generic.inc"
-;
-
-static const char* SKSL_MIXER_INCLUDE =
-#include "sksl_mixer.inc"
 ;
 
 namespace SkSL {
@@ -1265,18 +1261,12 @@ std::unique_ptr<Program> Compiler::convertProgram(Program::Kind kind, String tex
                                          &elements);
             fIRGenerator->fSymbolTable->markAllFunctionsBuiltin();
             break;
-        case Program::kPipelineStage_Kind:
+        case Program::kPipelineStage_Kind: // fall through
+        case Program::kGeneric_Kind:
             inherited = nullptr;
             fIRGenerator->start(&settings, nullptr);
-            fIRGenerator->convertProgram(kind, SKSL_PIPELINE_STAGE_INCLUDE,
+            fIRGenerator->convertProgram(kind, SKSL_GENERIC_INCLUDE,
-                                         strlen(SKSL_PIPELINE_STAGE_INCLUDE), *fTypes, &elements);
+                                         strlen(SKSL_GENERIC_INCLUDE), *fTypes, &elements);
-            fIRGenerator->fSymbolTable->markAllFunctionsBuiltin();
-            break;
-        case Program::kMixer_Kind:
-            inherited = nullptr;
-            fIRGenerator->start(&settings, nullptr);
-            fIRGenerator->convertProgram(kind, SKSL_MIXER_INCLUDE, strlen(SKSL_MIXER_INCLUDE),
-                                         *fTypes, &elements);
             fIRGenerator->fSymbolTable->markAllFunctionsBuiltin();
             break;
     }

src/sksl/SkSLIRGenerator.cpp

@@ -685,11 +685,6 @@ std::unique_ptr<Statement> IRGenerator::getNormalizeSkPositionCode() {
     return std::unique_ptr<Statement>(new ExpressionStatement(std::move(result)));
 }
 
-// returns true if the modifiers are (explicitly or implicitly) nothing but 'in'
-static bool is_in(const Modifiers& modifiers) {
-    return (modifiers.fFlags & ~Modifiers::kIn_Flag) == 0;
-}
-
 void IRGenerator::convertFunction(const ASTFunction& f) {
     const Type* returnType = this->convertType(*f.fReturnType);
     if (!returnType) {
@@ -744,19 +739,8 @@ void IRGenerator::convertFunction(const ASTFunction& f) {
                 }
                 break;
             }
-            case Program::kMixer_Kind: {
+            case Program::kGeneric_Kind:
-                if (*returnType != *fContext.fVoid_Type ||
-                    parameters.size() != 2 ||
-                    parameters[0]->fType != *fContext.fHalf4_Type ||
-                    !is_in(parameters[0]->fModifiers) ||
-                    parameters[1]->fType != *fContext.fHalf4_Type ||
-                    !is_in(parameters[1]->fModifiers)) {
-                    fErrors.error(f.fOffset, "mixer stage 'main' must be declared void main("
-                                             "half4, half4)");
-                    return;
-                }
                 break;
-            }
             default:
                 if (parameters.size()) {
                     fErrors.error(f.fOffset, "shader 'main' must have zero parameters");

src/sksl/SkSLInterpreter.cpp

@@ -29,7 +29,7 @@ namespace SkSL {
 
 static constexpr int UNINITIALIZED = 0xDEADBEEF;
 
-Interpreter::Value Interpreter::run(const ByteCodeFunction& f, Interpreter::Value args[],
+Interpreter::Value* Interpreter::run(const ByteCodeFunction& f, Interpreter::Value args[],
                                     Interpreter::Value inputs[]) {
     fIP = 0;
     fCurrentFunction = &f;
@@ -62,7 +62,7 @@ Interpreter::Value Interpreter::run(const ByteCodeFunction& f, Interpreter::Valu
             offset += p->fType.columns() * p->fType.rows();
         }
     }
-    return fReturnValue;
+    return fStack.data();
 }
 
 struct CallbackCtx : public SkRasterPipeline_CallbackCtx {
@@ -142,7 +142,7 @@ void Interpreter::disassemble(const ByteCodeFunction& f) {
             case ByteCodeInstruction::kDupDown: printf("dupdown %d", this->read8()); break;
             case ByteCodeInstruction::kFloatToInt: printf("floattoint"); break;
             case ByteCodeInstruction::kLoad: printf("load"); break;
-            case ByteCodeInstruction::kLoadGlobal: printf("loadglobal"); break;
+            case ByteCodeInstruction::kLoadGlobal: printf("loadglobal %d", this->read8()); break;
             case ByteCodeInstruction::kLoadSwizzle: {
                 int count = this->read8();
                 printf("loadswizzle %d", count);
@@ -166,8 +166,10 @@ void Interpreter::disassemble(const ByteCodeFunction& f) {
                 break;
             case ByteCodeInstruction::kRemainderS: printf("remainders"); break;
             case ByteCodeInstruction::kRemainderU: printf("remainderu"); break;
+            case ByteCodeInstruction::kReturn: printf("return %d", this->read8()); break;
             case ByteCodeInstruction::kSignedToFloat: printf("signedtofloat"); break;
             case ByteCodeInstruction::kStore: printf("store"); break;
+            case ByteCodeInstruction::kStoreGlobal: printf("storeglobal"); break;
             case ByteCodeInstruction::kStoreSwizzle: {
                 int count = this->read8();
                 printf("storeswizzle %d", count);
@@ -189,7 +191,8 @@ void Interpreter::disassemble(const ByteCodeFunction& f) {
             }
             case ByteCodeInstruction::kUnsignedToFloat: printf("unsignedtofloat"); break;
             case ByteCodeInstruction::kVector: printf("vector%d", this->read8()); break;
-            default: SkASSERT(false);
+            default: printf("%d\n", fCurrentFunction->fCode[fIP - 1]);
+                     SkASSERT(false);
         }
         printf("\n");
     }
@@ -323,6 +326,14 @@ void Interpreter::next() {
             break;
         BINARY_OP(kRemainderS, int32_t, fSigned, %)
         BINARY_OP(kRemainderU, uint32_t, fUnsigned, %)
+        case ByteCodeInstruction::kReturn: {
+            int count = this->read8();
+            for (int i = 0; i < count; ++i) {
+                fStack[i] = fStack[fStack.size() - count + i];
+            }
+            fIP = (int) fCurrentFunction->fCode.size();
+            break;
+        }
         case ByteCodeInstruction::kStore: {
             Value value = this->pop();
             int target = this->pop().fSigned;

src/sksl/SkSLInterpreter.h

@@ -53,14 +53,13 @@ public:
 
     Interpreter(std::unique_ptr<Program> program, std::unique_ptr<ByteCode> byteCode)
     : fProgram(std::move(program))
-    , fByteCode(std::move(byteCode))
+    , fByteCode(std::move(byteCode)) {}
-    , fReturnValue(0) {}
 
     /**
      * Invokes the specified function with the given arguments, returning its return value. 'out'
      * and 'inout' parameters will result in the 'args' array being modified.
      */
-    Value run(const ByteCodeFunction& f, Value args[], Value inputs[]);
+    Value* run(const ByteCodeFunction& f, Value args[], Value inputs[]);
 
 private:
     StackIndex stackAlloc(int count);
@@ -93,7 +92,6 @@ private:
     const ByteCodeFunction* fCurrentFunction;
     std::vector<Value> fGlobals;
     std::vector<Value> fStack;
-    Value fReturnValue;
 };
 
 } // namespace

src/sksl/SkSLPipelineStageCodeGenerator.cpp

@@ -156,19 +156,6 @@ void PipelineStageCodeGenerator::writeVariableReference(const VariableReference&
                 SkASSERT(found);
                 fFormatArgs->push_back(Compiler::FormatArg(Compiler::FormatArg::Kind::kUniform,
                                                            index));
-            } else if (fProgramKind == Program::kMixer_Kind &&
-                     ref.fVariable.fStorage == Variable::kParameter_Storage &&
-                     fCurrentFunction->fName == "main") {
-                this->write("%s");
-                for (size_t i = 0; i < fCurrentFunction->fParameters.size(); ++i) {
-                    if (fCurrentFunction->fParameters[i] == &ref.fVariable) {
-                        fFormatArgs->push_back(Compiler::FormatArg(
-                                                         Compiler::FormatArg::Kind::kChildProcessor,
-                                                         i));
-                        return;
-                    }
-                }
-                SkASSERT(false);
             } else {
                 this->write(ref.fVariable.fName);
             }

src/sksl/ir/SkSLProgram.h

@@ -214,7 +214,8 @@ struct Program {
         kGeometry_Kind,
         kFragmentProcessor_Kind,
         kPipelineStage_Kind,
-        kMixer_Kind
+        kGeneric_Kind,
+
     };
 
     Program(Kind kind,

tests/SkSLInterpreterTest.cpp

@@ -10,12 +10,53 @@
 
 #include "Test.h"
 
+void test(skiatest::Reporter* r, const char* src, SkSL::Interpreter::Value* in, int expectedCount,
+          SkSL::Interpreter::Value* expected) {
+    SkSL::Compiler compiler;
+    SkSL::Program::Settings settings;
+    std::unique_ptr<SkSL::Program> program = compiler.convertProgram(
+                                                             SkSL::Program::kGeneric_Kind,
+                                                             SkSL::String(src), settings);
+    REPORTER_ASSERT(r, program);
+    if (program) {
+        std::unique_ptr<SkSL::ByteCode> byteCode = compiler.toByteCode(*program);
+        REPORTER_ASSERT(r, !compiler.errorCount());
+        if (compiler.errorCount() > 0) {
+            printf("%s\n%s", src, compiler.errorText().c_str());
+            return;
+        }
+        SkSL::ByteCodeFunction* main = byteCode->fFunctions[0].get();
+        SkSL::Interpreter interpreter(std::move(program), std::move(byteCode));
+        SkSL::Interpreter::Value* out = interpreter.run(*main, in, nullptr);
+        bool valid = !memcmp(out, expected, sizeof(SkSL::Interpreter::Value) * expectedCount);
+        if (!valid) {
+            printf("for program: %s\n", src);
+            printf("    expected (");
+            const char* separator = "";
+            for (int i = 0; i < expectedCount; ++i) {
+                printf("%s%f", separator, expected[i].fFloat);
+                separator = ", ";
+            }
+            printf("), but received (");
+            separator = "";
+            for (int i = 0; i < expectedCount; ++i) {
+                printf("%s%f", separator, out[i].fFloat);
+                separator = ", ";
+            }
+            printf(")\n");
+        }
+        REPORTER_ASSERT(r, valid);
+    } else {
+        printf("%s\n%s", src, compiler.errorText().c_str());
+    }
+}
+
 void test(skiatest::Reporter* r, const char* src, float inR, float inG, float inB, float inA,
         float expectedR, float expectedG, float expectedB, float expectedA) {
     SkSL::Compiler compiler;
     SkSL::Program::Settings settings;
     std::unique_ptr<SkSL::Program> program = compiler.convertProgram(
-                                                             SkSL::Program::kPipelineStage_Kind,
+                                                             SkSL::Program::kGeneric_Kind,
                                                              SkSL::String(src), settings);
     REPORTER_ASSERT(r, program);
     if (program) {
@@ -45,11 +86,6 @@ void test(skiatest::Reporter* r, const char* src, float inR, float inG, float in
     }
 }
 
-DEF_TEST(SkSLInterpreterTEMP_TEST, r) {
-    test(r, "void main(inout half4 color) { half4 c = color; color += c; }", 0.25, 0.5, 0.75, 1,
-         0.5, 1, 1.5, 2);
-}
-
 DEF_TEST(SkSLInterpreterAdd, r) {
     test(r, "void main(inout half4 color) { color.r = color.r + color.g; }", 0.25, 0.75, 0, 0, 1,
          0.75, 0, 0);
@@ -198,3 +234,15 @@ DEF_TEST(SkSLInterpreterGlobal, r) {
     test(r, "int x; void main(inout half4 color) { x = 10; color.b = x; }", 1, 2, 3, 4, 1, 2, 10,
          4);
 }
+
+DEF_TEST(SkSLInterpreterGeneric, r) {
+    float value1 = 5;
+    float expected1 = 25;
+    test(r, "float main(float x) { return x * x; }", (SkSL::Interpreter::Value*) &value1, 1,
+         (SkSL::Interpreter::Value*) &expected1);
+    float value2[2] = { 5, 25 };
+    float expected2[2] = { 25, 625 };
+    test(r, "float2 main(float x, float y) { return float2(x * x, y * y); }",
+         (SkSL::Interpreter::Value*) &value2, 2,
+         (SkSL::Interpreter::Value*) expected2);
+}