From 8feeff929e57ea63914213f3b14d8f00b287a0ad Mon Sep 17 00:00:00 2001 From: Ethan Nicholas Date: Thu, 30 Mar 2017 14:11:58 -0400 Subject: [PATCH] fixed skslc SPIR-V memory error BUG=skia:6446 Change-Id: Ibc55124db60d6a05964ddcd02d285e313379f93e Reviewed-on: https://skia-review.googlesource.com/10756 Reviewed-by: Greg Daniel Commit-Queue: Ethan Nicholas --- src/sksl/SkSLCompiler.cpp | 4 ++-- src/sksl/SkSLIRGenerator.cpp | 5 +++-- src/sksl/SkSLSPIRVCodeGenerator.cpp | 10 +++++++--- src/sksl/SkSLSPIRVCodeGenerator.h | 5 ++++- src/sksl/ir/SkSLInterfaceBlock.h | 4 ++-- src/sksl/ir/SkSLSymbolTable.h | 8 ++++---- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 1cbc4410b5..08b49e622a 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -49,8 +49,8 @@ namespace SkSL { Compiler::Compiler() : fErrorCount(0) { - auto types = std::shared_ptr(new SymbolTable(*this)); - auto symbols = std::shared_ptr(new SymbolTable(types, *this)); + auto types = std::shared_ptr(new SymbolTable(this)); + auto symbols = std::shared_ptr(new SymbolTable(types, this)); fIRGenerator = new IRGenerator(&fContext, symbols, *this); fTypes = types; #define ADD_TYPE(t) types->addWithoutOwnership(fContext.f ## t ## _Type->fName, \ diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index ae2a90f899..2a4c85dd7c 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -108,7 +108,7 @@ IRGenerator::IRGenerator(const Context* context, std::shared_ptr sy , fErrors(errorReporter) {} void IRGenerator::pushSymbolTable() { - fSymbolTable.reset(new SymbolTable(std::move(fSymbolTable), fErrors)); + fSymbolTable.reset(new SymbolTable(std::move(fSymbolTable), &fErrors)); } void IRGenerator::popSymbolTable() { @@ -665,7 +665,8 @@ std::unique_ptr IRGenerator::convertInterfaceBlock(const ASTInte (int) i))); } } - return std::unique_ptr(new InterfaceBlock(intf.fPosition, *var, + return std::unique_ptr(new InterfaceBlock(intf.fPosition, + var, intf.fTypeName, intf.fInstanceName, std::move(sizes), diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp index 93ec4ce9b2..2fa4e648f2 100644 --- a/src/sksl/SkSLSPIRVCodeGenerator.cpp +++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp @@ -1865,7 +1865,7 @@ SpvId SPIRVCodeGenerator::writeVariableReference(const VariableReference& ref, S // need to remap to a top-left coordinate system if (fRTHeightStructId == (SpvId) -1) { // height variable hasn't been written yet - std::shared_ptr st(new SymbolTable(fErrors)); + std::shared_ptr st(new SymbolTable(&fErrors)); ASSERT(fRTHeightFieldIndex == (SpvId) -1); std::vector fields; fields.emplace_back(Modifiers(), SkString(SKSL_RTHEIGHT_NAME), @@ -1874,8 +1874,12 @@ SpvId SPIRVCodeGenerator::writeVariableReference(const VariableReference& ref, S Type intfStruct(Position(), name, fields); Layout layout(-1, -1, 1, -1, -1, -1, -1, false, false, false, Layout::Format::kUnspecified, false, Layout::kUnspecified_Primitive, -1, -1); - Variable intfVar(Position(), Modifiers(layout, Modifiers::kUniform_Flag), name, - intfStruct, Variable::kGlobal_Storage); + Variable* intfVar = new Variable(Position(), + Modifiers(layout, Modifiers::kUniform_Flag), + name, + intfStruct, + Variable::kGlobal_Storage); + fSynthetics.takeOwnership(intfVar); InterfaceBlock intf(Position(), intfVar, name, SkString(""), std::vector>(), st); fRTHeightStructId = this->writeInterfaceBlock(intf); diff --git a/src/sksl/SkSLSPIRVCodeGenerator.h b/src/sksl/SkSLSPIRVCodeGenerator.h index 1cdc653c85..ea160b195d 100644 --- a/src/sksl/SkSLSPIRVCodeGenerator.h +++ b/src/sksl/SkSLSPIRVCodeGenerator.h @@ -74,7 +74,8 @@ public: , fBoolTrue(0) , fBoolFalse(0) , fSetupFragPosition(false) - , fCurrentBlock(0) { + , fCurrentBlock(0) + , fSynthetics(nullptr, errors) { this->setupIntrinsics(); } @@ -299,6 +300,8 @@ private: std::stack fContinueTarget; SpvId fRTHeightStructId = (SpvId) -1; SpvId fRTHeightFieldIndex = (SpvId) -1; + // holds variables synthesized during output, for lifetime purposes + SymbolTable fSynthetics; friend class PointerLValue; friend class SwizzleLValue; diff --git a/src/sksl/ir/SkSLInterfaceBlock.h b/src/sksl/ir/SkSLInterfaceBlock.h index 0de37c59e8..0ec33e3d9c 100644 --- a/src/sksl/ir/SkSLInterfaceBlock.h +++ b/src/sksl/ir/SkSLInterfaceBlock.h @@ -25,11 +25,11 @@ namespace SkSL { * At the IR level, this is represented by a single variable of struct type. */ struct InterfaceBlock : public ProgramElement { - InterfaceBlock(Position position, const Variable& var, SkString typeName, SkString instanceName, + InterfaceBlock(Position position, const Variable* var, SkString typeName, SkString instanceName, std::vector> sizes, std::shared_ptr typeOwner) : INHERITED(position, kInterfaceBlock_Kind) - , fVariable(std::move(var)) + , fVariable(*var) , fTypeName(std::move(typeName)) , fInstanceName(std::move(instanceName)) , fSizes(std::move(sizes)) diff --git a/src/sksl/ir/SkSLSymbolTable.h b/src/sksl/ir/SkSLSymbolTable.h index df8dc713ae..948a447b80 100644 --- a/src/sksl/ir/SkSLSymbolTable.h +++ b/src/sksl/ir/SkSLSymbolTable.h @@ -24,12 +24,12 @@ struct FunctionDeclaration; */ class SymbolTable { public: - SymbolTable(ErrorReporter& errorReporter) - : fErrorReporter(errorReporter) {} + SymbolTable(ErrorReporter* errorReporter) + : fErrorReporter(*errorReporter) {} - SymbolTable(std::shared_ptr parent, ErrorReporter& errorReporter) + SymbolTable(std::shared_ptr parent, ErrorReporter* errorReporter) : fParent(parent) - , fErrorReporter(errorReporter) {} + , fErrorReporter(*errorReporter) {} const Symbol* operator[](const SkString& name);