Remove automatic name mangling from DSL.

This feature is always disabled when DSLParser is used. Name mangling
was only useful when compiling hand-authored DSL code that explicitly
declared two variables with the same name in separate scopes. In
practice, as long as we aren't declaring complete programs in DSL form,
we don't need this.

Change-Id: Icad921eb86365b3b114ff1872b1c40c41470a4b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541637
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2022-05-17 22:55:18 -04:00 committed by SkCQ
parent 6b0e8256aa
commit d35e9151bc
12 changed files with 8 additions and 68 deletions

View File

@ -70,7 +70,6 @@ public:
, fCompiler(fCaps.shaderCaps())
, fOutput(output) {
fSettings.fOptimize = optimize;
fSettings.fDSLMangling = false;
// The test programs we compile don't follow Vulkan rules and thus produce invalid
// SPIR-V. This is harmless, so long as we don't try to validate them.
fSettings.fValidateSPIRV = false;

View File

@ -137,7 +137,6 @@ protected:
std::unique_ptr<SkSL::Statement> fDeclaration;
const SkSL::Variable* fVar = nullptr;
Position fNamePosition;
std::string_view fRawName; // for error reporting
std::string_view fName;
DSLExpression fInitialValue;
// true if we have attempted to create the SkSL var

View File

@ -510,7 +510,6 @@ std::unique_ptr<Program> Compiler::convertProgram(ProgramKind kind,
this->resetErrors();
fInliner.reset();
settings.fDSLMangling = false;
return DSLParser(this, settings, kind, std::move(text)).program();
}

View File

@ -112,8 +112,6 @@ DSLParser::DSLParser(Compiler* compiler, const ProgramSettings& settings, Progra
// We don't want to have to worry about manually releasing all of the objects in the event that
// an error occurs
fSettings.fAssertDSLObjectsReleased = false;
// We manage our symbol tables manually, so no need for name mangling
fSettings.fDSLMangling = false;
fLexer.start(*fText);
}

View File

@ -75,8 +75,6 @@ struct ProgramSettings {
// every temporary value, even ones that would otherwise be optimized away entirely. The other
// debug opcodes are much less invasive on the generated code.
bool fAllowTraceVarInSkVMDebugTrace = true;
// If true, the DSL should automatically mangle symbol names.
bool fDSLMangling = true;
// If true, the DSL should automatically mark variables declared upon creation.
bool fDSLMarkVarsDeclared = false;
// If true, the DSL should install a memory pool when possible.

View File

@ -223,7 +223,6 @@ generated_cc_atom(
"//src/sksl/ir:SkSLFieldAccess_hdr",
"//src/sksl/ir:SkSLFunctionCall_hdr",
"//src/sksl/ir:SkSLSymbolTable_hdr",
"//src/sksl/ir:SkSLType_hdr",
"//src/sksl/ir:SkSLVariable_hdr",
],
)

View File

@ -70,7 +70,7 @@ void DSLFunction::init(DSLModifiers modifiers, const DSLType& returnType, std::s
pos,
modifiers.fPosition,
ThreadContext::Modifiers(modifiers.fModifiers),
name == "main" ? name : DSLWriter::Name(name),
name,
std::move(paramVars), returnType.fPosition,
&returnType.skslType());
ThreadContext::ReportErrors(pos);

View File

@ -22,7 +22,6 @@
#include "src/sksl/ir/SkSLFieldAccess.h"
#include "src/sksl/ir/SkSLFunctionCall.h"
#include "src/sksl/ir/SkSLSymbolTable.h"
#include "src/sksl/ir/SkSLType.h"
#include "src/sksl/ir/SkSLVariable.h"
#include <string>
@ -47,8 +46,7 @@ DSLVarBase::DSLVarBase(const DSLModifiers& modifiers, DSLType type, std::string_
: fModifiers(std::move(modifiers))
, fType(std::move(type))
, fNamePosition(namePos)
, fRawName(name)
, fName(fType.skslType().isOpaque() ? name : DSLWriter::Name(name))
, fName(name)
, fInitialValue(std::move(initialValue))
, fDeclared(DSLWriter::MarkVarsDeclared())
, fPosition(pos) {}
@ -57,8 +55,8 @@ DSLVarBase::~DSLVarBase() {
if (fDeclaration && !fDeclared) {
ThreadContext::ReportError(String::printf("variable '%.*s' was destroyed without being "
"declared",
(int)fRawName.length(),
fRawName.data()).c_str());
(int)fName.length(),
fName.data()).c_str());
}
}
@ -70,7 +68,6 @@ void DSLVarBase::swap(DSLVarBase& other) {
std::swap(fDeclaration, other.fDeclaration);
std::swap(fVar, other.fVar);
std::swap(fNamePosition, other.fNamePosition);
std::swap(fRawName, other.fRawName);
std::swap(fName, other.fName);
std::swap(fInitialValue.fExpression, other.fInitialValue.fExpression);
std::swap(fDeclared, other.fDeclared);

View File

@ -34,7 +34,6 @@ generated_cc_atom(
"//include/sksl:DSLType_hdr",
"//include/sksl:DSLVar_hdr",
"//include/sksl:SkSLPosition_hdr",
"//src/sksl:SkSLMangler_hdr",
"//src/sksl:SkSLModifiersPool_hdr",
"//src/sksl:SkSLProgramSettings_hdr",
"//src/sksl:SkSLThreadContext_hdr",

View File

@ -19,7 +19,6 @@
#include "include/sksl/DSLType.h"
#include "include/sksl/DSLVar.h"
#include "include/sksl/SkSLPosition.h"
#include "src/sksl/SkSLMangler.h"
#include "src/sksl/SkSLModifiersPool.h"
#include "src/sksl/SkSLProgramSettings.h"
#include "src/sksl/SkSLThreadContext.h"
@ -30,8 +29,6 @@
#include "src/sksl/ir/SkSLVarDeclarations.h"
#include "src/sksl/ir/SkSLVariable.h"
#include <string>
#include <type_traits>
#include <utility>
#include <vector>
@ -39,20 +36,6 @@ namespace SkSL {
namespace dsl {
bool DSLWriter::ManglingEnabled() {
return ThreadContext::Instance().fSettings.fDSLMangling;
}
std::string_view DSLWriter::Name(std::string_view name) {
if (ManglingEnabled()) {
const std::string* s = ThreadContext::SymbolTable()->takeOwnershipOfString(
ThreadContext::Instance().fMangler.uniqueName(name,
ThreadContext::SymbolTable().get()));
return s->c_str();
}
return name;
}
const SkSL::Variable* DSLWriter::Var(DSLVarBase& var) {
// fInitialized is true if we have attempted to create a var, whether or not we actually
// succeeded. If it's true, we don't want to try again, to avoid reporting the same error

View File

@ -15,7 +15,6 @@
#endif // !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU
#include <memory>
#include <string_view>
namespace SkSL {
@ -35,28 +34,6 @@ class DSLVar;
*/
class DSLWriter {
public:
/**
* Returns whether name mangling is enabled. Mangling is important for the DSL because its
* variables normally all go into the same symbol table; for instance if you were to translate
* this legal (albeit silly) GLSL code:
* int x;
* {
* int x;
* }
*
* into DSL, you'd end up with:
* DSLVar x1(kInt_Type, "x");
* DSLVar x2(kInt_Type, "x");
* Declare(x1);
* Block(Declare(x2));
*
* with x1 and x2 ending up in the same symbol table. This is fine as long as their effective
* names are different, so mangling prevents this situation from causing problems.
*/
static bool ManglingEnabled();
static std::string_view Name(std::string_view name);
/**
* Returns the SkSL variable corresponding to a DSL var.
*/

View File

@ -55,7 +55,6 @@ using namespace SkSL::dsl;
SkSL::ProgramSettings default_settings() {
SkSL::ProgramSettings result;
result.fDSLMarkVarsDeclared = true;
result.fDSLMangling = false;
return result;
}
@ -66,11 +65,10 @@ SkSL::ProgramSettings no_mark_vars_declared() {
}
/**
* In addition to issuing an automatic Start() and End(), disables mangling and optionally
* auto-declares variables during its lifetime. Variable auto-declaration simplifies testing so we
* don't have to sprinkle all the tests with a bunch of Declare(foo).release() calls just to avoid
* errors, especially given that some of the variables have options that make them an error to
* actually declare.
* Issues an automatic Start() and End(), and optionally auto-declares variables during its
* lifetime. Variable auto-declaration simplifies testing so we don't have to sprinkle all the tests
* with a bunch of Declare(foo).release() calls just to avoid errors, especially given that some of
* the variables have options that make them an error to actually declare.
*/
class AutoDSLContext {
public:
@ -203,12 +201,6 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFlags, r, ctxInfo) {
Var y(kFloat_Type, "y");
EXPECT_EQUAL(x = y, "(x = half(y))");
}
{
AutoDSLContext context(ctxInfo.directContext()->priv().getGpu(), SkSL::ProgramSettings());
Var x(kInt_Type, "x");
EXPECT_EQUAL(Declare(x), "int _0_x;");
}
}
DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFloat, r, ctxInfo) {