Moved DSLWriter::ReleaseProgram into DSLCore.

This makes the API accessible from the upcoming DSLParser.

Change-Id: I0ee31c7648e35dfeaa50285cf93dcdb9dd4872bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/417678
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>

include/sksl/DSLCore.h

@@ -23,6 +23,7 @@
 namespace SkSL {
 
 class Compiler;
+struct Program;
 struct ProgramSettings;
 
 namespace dsl {
@@ -47,6 +48,11 @@ void Start(SkSL::Compiler* compiler, SkSL::ProgramKind kind, const SkSL::Program
  */
 void End();
 
+/**
+ * Returns all global elements (functions and global variables) as a self-contained Program.
+ */
+std::unique_ptr<SkSL::Program> ReleaseProgram();
+
 /**
  * Installs an ErrorHandler which will be notified of any errors that occur during DSL calls. If
  * no ErrorHandler is installed, any errors will be fatal.
@@ -68,7 +74,7 @@ DSLStatement Break();
 DSLStatement Continue();
 
 /**
- * Creates a variable declaration statement.
+ * Creates a local variable declaration statement.
  */
 DSLStatement Declare(DSLVar& var, PositionInfo pos = PositionInfo());
 

src/sksl/SkSLCompiler.cpp

@@ -487,8 +487,8 @@ std::unique_ptr<Program> Compiler::convertProgram(
     dsl::Start(this, kind, settings);
     IRGenerator::IRBundle ir = fIRGenerator->convertProgram(baseModule, /*isBuiltinCode=*/false,
                                                             textPtr->c_str(), textPtr->size());
-    // Ideally, we would just use DSLWriter::ReleaseProgram and not have to do any manual mucking
-    // about with the memory pool, but we've got some impedance mismatches to solve first
+    // Ideally, we would just use dsl::ReleaseProgram and not have to do any manual mucking about
+    // with the memory pool, but we've got some impedance mismatches to solve first
     Pool* memoryPool = dsl::DSLWriter::MemoryPool().get();
     auto program = std::make_unique<Program>(std::move(textPtr),
                                              std::move(dsl::DSLWriter::GetProgramConfig()),

src/sksl/SkSLCompiler.h

@@ -44,6 +44,7 @@ class SkSLCompileBench;
 namespace SkSL {
 
 namespace dsl {
+    class DSLCore;
     class DSLWriter;
 }
 
@@ -265,6 +266,7 @@ private:
 
     friend class AutoSource;
     friend class ::SkSLCompileBench;
+    friend class dsl::DSLCore;
     friend class dsl::DSLWriter;
 };
 

src/sksl/dsl/DSLCore.cpp

@@ -53,6 +53,41 @@ void SetErrorHandler(ErrorHandler* errorHandler) {
 
 class DSLCore {
 public:
+    static std::unique_ptr<SkSL::Program> ReleaseProgram() {
+        DSLWriter& instance = DSLWriter::Instance();
+        SkSL::IRGenerator& ir = DSLWriter::IRGenerator();
+        IRGenerator::IRBundle bundle = ir.finish();
+        Pool* pool = DSLWriter::Instance().fPool.get();
+        auto result = std::make_unique<SkSL::Program>(/*source=*/nullptr,
+                                                      std::move(instance.fConfig),
+                                                      DSLWriter::Instance().fCompiler->fContext,
+                                                      std::move(bundle.fElements),
+                                                      std::move(bundle.fSharedElements),
+                                                      std::move(instance.fModifiersPool),
+                                                      std::move(bundle.fSymbolTable),
+                                                      std::move(instance.fPool),
+                                                      bundle.fInputs);
+        bool success = false;
+        if (DSLWriter::Compiler().errorCount() || DSLWriter::Instance().fEncounteredErrors) {
+            // Make sure that if we encountered any compiler errors, we reported them through the
+            // DSL error handling side of things.
+            SkASSERT(!DSLWriter::Compiler().errorCount() ||
+                     DSLWriter::Instance().fEncounteredErrors);
+            // Do not return programs that failed to compile.
+        } else if (!DSLWriter::Compiler().optimize(*result)) {
+            // Do not return programs that failed to optimize.
+        } else {
+            // We have a successful program!
+            success = true;
+        }
+        if (pool) {
+            pool->detachFromThread();
+        }
+        SkASSERT(DSLWriter::ProgramElements().empty());
+        SkASSERT(!DSLWriter::SymbolTable());
+        return success ? std::move(result) : nullptr;
+    }
+
     static DSLVar sk_FragColor() {
         return DSLVar("sk_FragColor");
     }
@@ -208,6 +243,10 @@ public:
     }
 };
 
+std::unique_ptr<SkSL::Program> ReleaseProgram() {
+    return DSLCore::ReleaseProgram();
+}
+
 DSLVar sk_FragColor() {
     return DSLCore::sk_FragColor();
 }

src/sksl/dsl/DSLRuntimeEffects.cpp

@@ -29,13 +29,13 @@ void StartRuntimeShader(SkSL::Compiler* compiler) {
 }
 
 sk_sp<SkRuntimeEffect> EndRuntimeShader() {
-    std::unique_ptr<SkSL::Program> program = DSLWriter::ReleaseProgram();
-    auto result = SkRuntimeEffect::MakeForShader(std::move(program));
-    // TODO(skbug.com/11862): propagate errors properly
-    SkASSERTF(result.effect, "%s\n", result.errorText.c_str());
-    SkSL::ProgramSettings& settings = DSLWriter::IRGenerator().fContext.fConfig->fSettings;
-    settings.fInlineThreshold = SkSL::kDefaultInlineThreshold;
-    settings.fAllowNarrowingConversions = false;
+    std::unique_ptr<SkSL::Program> program = ReleaseProgram();
+    SkRuntimeEffect::Result result;
+    if (program) {
+        result = SkRuntimeEffect::MakeForShader(std::move(program));
+        // TODO(skbug.com/11862): propagate errors properly
+        SkASSERTF(result.effect, "%s\n", result.errorText.c_str());
+    }
     End();
     return result.effect;
 }

src/sksl/dsl/priv/DSLWriter.cpp

@@ -208,6 +208,7 @@ DSLPossibleStatement DSLWriter::ConvertSwitch(std::unique_ptr<Expression> value,
 }
 
 void DSLWriter::ReportError(const char* msg, PositionInfo* info) {
+    Instance().fEncounteredErrors = true;
     if (info && !info->file_name()) {
         info = nullptr;
     }
@@ -264,28 +265,6 @@ void DSLWriter::MarkDeclared(DSLVar& var) {
     var.fDeclared = true;
 }
 
-std::unique_ptr<SkSL::Program> DSLWriter::ReleaseProgram() {
-    DSLWriter& instance = Instance();
-    SkSL::IRGenerator& ir = IRGenerator();
-    IRGenerator::IRBundle bundle = ir.finish();
-    Pool* pool = Instance().fPool.get();
-    auto result = std::make_unique<SkSL::Program>(/*source=*/nullptr,
-                                                  std::move(instance.fConfig),
-                                                  Compiler().fContext,
-                                                  std::move(bundle.fElements),
-                                                  std::move(bundle.fSharedElements),
-                                                  std::move(instance.fModifiersPool),
-                                                  std::move(bundle.fSymbolTable),
-                                                  std::move(instance.fPool),
-                                                  bundle.fInputs);
-    if (pool) {
-        pool->detachFromThread();
-    }
-    SkASSERT(ProgramElements().empty());
-    SkASSERT(!SymbolTable());
-    return result;
-}
-
 #if SKSL_USE_THREAD_LOCAL
 
 thread_local DSLWriter* instance = nullptr;

src/sksl/dsl/priv/DSLWriter.h

@@ -256,8 +256,6 @@ public:
         return Instance().fSettings.fDSLMarkVarsDeclared;
     }
 
-    static std::unique_ptr<SkSL::Program> ReleaseProgram();
-
     static DSLWriter& Instance();
 
     static void SetInstance(std::unique_ptr<DSLWriter> instance);
@@ -274,6 +272,7 @@ private:
     ErrorHandler* fErrorHandler = nullptr;
     ProgramSettings fSettings;
     Mangler fMangler;
+    bool fEncounteredErrors = false;
 #if !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU
     struct StackFrame {
         GrGLSLFragmentProcessor* fProcessor;

tests/SkDSLRuntimeEffectTest.cpp

@@ -36,10 +36,12 @@ public:
         StartRuntimeShader(fCompiler.get());
     }
 
-    void end() {
+    void end(bool expectSuccess = true) {
         sk_sp<SkRuntimeEffect> effect = EndRuntimeShader();
-        SkASSERT(effect);
-        fBuilder.init(std::move(effect));
+        REPORTER_ASSERT(fReporter, effect ? expectSuccess : !expectSuccess);
+        if (effect) {
+            fBuilder.init(std::move(effect));
+        }
     }
 
     SkRuntimeShaderBuilder::BuilderUniform uniform(const char* name) {
@@ -191,6 +193,27 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext
         effect.test(0xFF000000, 0xFF0000FF, 0xFF00FF00, 0xFF00FFFF);
     }
 
+    // Test error reporting. We put this before a couple of successful tests to ensure that a
+    // failure doesn't leave us in a broken state.
+    {
+        class SimpleErrorHandler : public ErrorHandler {
+        public:
+            void handleError(const char* msg, PositionInfo* pos) override {
+                fMsg = msg;
+            }
+
+            SkSL::String fMsg;
+        } errorHandler;
+        effect.start();
+        SetErrorHandler(&errorHandler);
+        Var p(kFloat2_Type, "p");
+        Function(kHalf4_Type, "main", p).define(
+            Return(1) // Error, type mismatch
+        );
+        effect.end(false);
+        REPORTER_ASSERT(r, errorHandler.fMsg == "error: expected 'half4', but found 'int'\n");
+    }
+
     // Mutating coords should work. (skbug.com/10918)
     {
         effect.start();