Allow Mangler to be used outside of Inliner.
It now lives in the Compiler instead of the Inliner, and we have a
pointer to it inside the Context. I also took the opportunity to remove
a few unnecessary #includes from Context and fixed up the fallout from
this.
POTENTIALLY IMPORTANT FOOTNOTE: DSLWriter has its own Mangler object
inside of it. I experimented with removing this and having it share the
Mangler object inside the Context, but this caused failures in
"testThreading" and "testpersistentcache" CQ bots; the Expected and
Actual images would mismatch. (The Expected would be missing some
draws.) This particularly affected some blur-related tests which are
likely to be GaussianConvolution--i.e. it uses DSL.
This might be a canary in the coalmine for some quirky DSL threading
issue? e.g. using the same Context on two threads at once?
Change-Id: I2290b810d9487c40a3dbef119c95d1572b14a5ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449357
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 257706f..f6800c2 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -124,7 +124,7 @@
Compiler::Compiler(const ShaderCapsClass* caps)
: fErrorReporter(this)
- , fContext(std::make_shared<Context>(fErrorReporter, *caps))
+ , fContext(std::make_shared<Context>(fErrorReporter, *caps, fMangler))
, fInliner(fContext.get()) {
SkASSERT(caps);
fRootModule.fSymbols = this->makeRootSymbolTable();
diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h
index 4380f6f..77d8e55 100644
--- a/src/sksl/SkSLCompiler.h
+++ b/src/sksl/SkSLCompiler.h
@@ -267,6 +267,7 @@
// holds ModifiersPools belonging to the core includes for lifetime purposes
ModifiersPool fCoreModifiers;
+ Mangler fMangler;
Inliner fInliner;
std::unique_ptr<IRGenerator> fIRGenerator;
diff --git a/src/sksl/SkSLContext.cpp b/src/sksl/SkSLContext.cpp
index 32f969e..88ecc0f 100644
--- a/src/sksl/SkSLContext.cpp
+++ b/src/sksl/SkSLContext.cpp
@@ -6,15 +6,18 @@
*/
#include "src/sksl/SkSLContext.h"
-
-#include "include/sksl/DSLCore.h"
-#include "src/sksl/dsl/priv/DSLWriter.h"
+#include "src/sksl/SkSLPool.h"
namespace SkSL {
-Context::Context(ErrorReporter& errors, const ShaderCapsClass& caps)
+Context::Context(ErrorReporter& errors, const ShaderCapsClass& caps, Mangler& mangler)
: fCaps(caps)
- , fErrors(&errors) {
+ , fErrors(&errors)
+ , fMangler(&mangler) {
+ SkASSERT(!Pool::IsAttached());
+}
+
+Context::~Context() {
SkASSERT(!Pool::IsAttached());
}
diff --git a/src/sksl/SkSLContext.h b/src/sksl/SkSLContext.h
index 75945f9..3284230 100644
--- a/src/sksl/SkSLContext.h
+++ b/src/sksl/SkSLContext.h
@@ -10,15 +10,15 @@
#include <memory>
-#include "include/sksl/SkSLErrorReporter.h"
#include "src/sksl/SkSLBuiltinTypes.h"
-#include "src/sksl/SkSLPool.h"
#include "src/sksl/SkSLUtil.h"
-#include "src/sksl/ir/SkSLExpression.h"
#include "src/sksl/ir/SkSLType.h"
namespace SkSL {
+class ErrorReporter;
+class Mangler;
+class ModifiersPool;
struct ProgramConfig;
/**
@@ -26,11 +26,8 @@
*/
class Context {
public:
- Context(ErrorReporter& errors, const ShaderCapsClass& caps);
-
- ~Context() {
- SkASSERT(!Pool::IsAttached());
- }
+ Context(ErrorReporter& errors, const ShaderCapsClass& caps, Mangler& mangler);
+ ~Context();
// The Context holds all of the built-in types.
BuiltinTypes fTypes;
@@ -44,8 +41,11 @@
// The Context holds a pointer to the configuration of the program being compiled.
ProgramConfig* fConfig = nullptr;
- // The Context holds a reference to our error reporter.
+ // The Context holds a pointer to our error reporter.
ErrorReporter* fErrors;
+
+ // The Context holds a pointer to the shared name-mangler.
+ Mangler* fMangler = nullptr;
};
} // namespace SkSL
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index 3c5465b..5c05901 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -275,7 +275,7 @@
}
void Inliner::reset() {
- fMangler.reset();
+ fContext->fMangler->reset();
fInlinedStatementCounter = 0;
}
@@ -556,7 +556,7 @@
// regard, but see `InlinerAvoidsVariableNameOverlap` for a counterexample where unique
// names are important.
const String* name = symbolTableForStatement->takeOwnershipOfString(
- fMangler.uniqueName(variable.name(), symbolTableForStatement));
+ fContext->fMangler->uniqueName(variable.name(), symbolTableForStatement));
auto clonedVar = std::make_unique<Variable>(
offset,
&variable.modifiers(),
@@ -597,8 +597,8 @@
SkASSERT(!(modifiers.fFlags & Modifiers::kOut_Flag));
// Provide our new variable with a unique name, and add it to our symbol table.
- const String* name =
- symbolTable->takeOwnershipOfString(fMangler.uniqueName(baseName, symbolTable));
+ const String* name = symbolTable->takeOwnershipOfString(
+ fContext->fMangler->uniqueName(baseName, symbolTable));
// Create our new variable and add it to the symbol table.
InlineVariable result;
diff --git a/src/sksl/SkSLInliner.h b/src/sksl/SkSLInliner.h
index 86345be..df872bb 100644
--- a/src/sksl/SkSLInliner.h
+++ b/src/sksl/SkSLInliner.h
@@ -117,7 +117,6 @@
ModifiersPool& modifiersPool() const { return *fContext->fModifiersPool; }
const Context* fContext = nullptr;
- Mangler fMangler;
int fInlinedStatementCounter = 0;
};
diff --git a/src/sksl/ir/SkSLDoStatement.cpp b/src/sksl/ir/SkSLDoStatement.cpp
index 0dc3485..8d159d0 100644
--- a/src/sksl/ir/SkSLDoStatement.cpp
+++ b/src/sksl/ir/SkSLDoStatement.cpp
@@ -5,10 +5,12 @@
* found in the LICENSE file.
*/
+#include "src/sksl/ir/SkSLDoStatement.h"
+
+#include "include/sksl/SkSLErrorReporter.h"
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLContext.h"
#include "src/sksl/SkSLProgramSettings.h"
-#include "src/sksl/ir/SkSLDoStatement.h"
namespace SkSL {
diff --git a/src/sksl/ir/SkSLPostfixExpression.cpp b/src/sksl/ir/SkSLPostfixExpression.cpp
index f4a1523..05758fc 100644
--- a/src/sksl/ir/SkSLPostfixExpression.cpp
+++ b/src/sksl/ir/SkSLPostfixExpression.cpp
@@ -5,9 +5,11 @@
* found in the LICENSE file.
*/
+#include "src/sksl/ir/SkSLPostfixExpression.h"
+
+#include "include/sksl/SkSLErrorReporter.h"
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLContext.h"
-#include "src/sksl/ir/SkSLPostfixExpression.h"
#include "src/sksl/ir/SkSLVariableReference.h"
namespace SkSL {
diff --git a/src/sksl/ir/SkSLTernaryExpression.cpp b/src/sksl/ir/SkSLTernaryExpression.cpp
index fc4240e..602a66a 100644
--- a/src/sksl/ir/SkSLTernaryExpression.cpp
+++ b/src/sksl/ir/SkSLTernaryExpression.cpp
@@ -5,12 +5,14 @@
* found in the LICENSE file.
*/
+#include "src/sksl/ir/SkSLTernaryExpression.h"
+
+#include "include/sksl/SkSLErrorReporter.h"
#include "src/sksl/SkSLConstantFolder.h"
#include "src/sksl/SkSLContext.h"
#include "src/sksl/SkSLOperators.h"
#include "src/sksl/SkSLProgramSettings.h"
#include "src/sksl/ir/SkSLLiteral.h"
-#include "src/sksl/ir/SkSLTernaryExpression.h"
namespace SkSL {
diff --git a/src/sksl/ir/SkSLVarDeclarations.cpp b/src/sksl/ir/SkSLVarDeclarations.cpp
index 725544c..f985fd7 100644
--- a/src/sksl/ir/SkSLVarDeclarations.cpp
+++ b/src/sksl/ir/SkSLVarDeclarations.cpp
@@ -5,10 +5,12 @@
* found in the LICENSE file.
*/
+#include "src/sksl/ir/SkSLVarDeclarations.h"
+
+#include "include/sksl/SkSLErrorReporter.h"
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLContext.h"
#include "src/sksl/SkSLProgramSettings.h"
-#include "src/sksl/ir/SkSLVarDeclarations.h"
namespace SkSL {
diff --git a/tests/SkSLMemoryLayoutTest.cpp b/tests/SkSLMemoryLayoutTest.cpp
index e6a5022..a6fb79d 100644
--- a/tests/SkSLMemoryLayoutTest.cpp
+++ b/tests/SkSLMemoryLayoutTest.cpp
@@ -7,14 +7,16 @@
#include "include/sksl/SkSLErrorReporter.h"
#include "src/sksl/SkSLContext.h"
+#include "src/sksl/SkSLMangler.h"
#include "src/sksl/SkSLMemoryLayout.h"
#include "tests/Test.h"
DEF_TEST(SkSLMemoryLayout140Test, r) {
- GrShaderCaps caps(GrContextOptions{});
SkSL::TestingOnly_AbortErrorReporter errors;
- SkSL::Context context(errors, caps);
+ GrShaderCaps caps(GrContextOptions{});
+ SkSL::Mangler mangler;
+ SkSL::Context context(errors, caps, mangler);
SkSL::MemoryLayout layout(SkSL::MemoryLayout::k140_Standard);
// basic types
@@ -99,9 +101,10 @@
}
DEF_TEST(SkSLMemoryLayout430Test, r) {
- GrShaderCaps caps(GrContextOptions{});
SkSL::TestingOnly_AbortErrorReporter errors;
- SkSL::Context context(errors, caps);
+ GrShaderCaps caps(GrContextOptions{});
+ SkSL::Mangler mangler;
+ SkSL::Context context(errors, caps, mangler);
SkSL::MemoryLayout layout(SkSL::MemoryLayout::k430_Standard);
// basic types
diff --git a/tests/SkSLTypeTest.cpp b/tests/SkSLTypeTest.cpp
index 338896d..e420d32 100644
--- a/tests/SkSLTypeTest.cpp
+++ b/tests/SkSLTypeTest.cpp
@@ -10,12 +10,14 @@
#include "include/sksl/SkSLErrorReporter.h"
#include "src/gpu/GrCaps.h"
#include "src/sksl/SkSLContext.h"
+#include "src/sksl/SkSLMangler.h"
#include "tests/Test.h"
DEF_TEST(SkSLTypeLimits, r) {
GrShaderCaps caps(GrContextOptions{});
SkSL::TestingOnly_AbortErrorReporter errors;
- SkSL::Context context(errors, caps);
+ SkSL::Mangler mangler;
+ SkSL::Context context(errors, caps, mangler);
using int_limits = std::numeric_limits<int32_t>;
REPORTER_ASSERT(r, context.fTypes.fInt->minimumValue() == int_limits::min());