Refactored DSL variable creation

Prior to this change, we were creating the SkSL variable and declaration
immediately on DSLVar creation. This causes problems with function
parameters, which are sometimes supposed to be tagged
SK_MAIN_COORDS_BUILTIN. If we have already created the variable and
inserted it into the symbol table, then by the time we determine the
variable is supposed to be SK_MAIN_COORDS_BUILTIN, it is too late to
modify the (now-const) variable.

We are not yet doing this tagging, but refactoring the creation in this
fashion paves the way to making it possible.

Change-Id: I031170502c5e7c1fff5ecfac01bea470ff4e61ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389216
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This commit is contained in:
Ethan Nicholas 2021-03-25 17:49:08 -04:00 committed by Skia Commit-Bot
parent 0c9d888748
commit 707d31578c
10 changed files with 70 additions and 41 deletions

View File

@ -60,7 +60,9 @@ public:
/**
* Creates an expression representing a variable reference.
*/
DSLExpression(const DSLVar& var);
DSLExpression(DSLVar& var);
DSLExpression(DSLVar&& var);
DSLExpression(DSLPossibleExpression expr, PositionInfo pos = PositionInfo());
@ -195,8 +197,6 @@ public:
DSLExpression field(const char* name, PositionInfo pos = PositionInfo());
DSLPossibleExpression operator=(const DSLVar& var);
DSLPossibleExpression operator=(DSLExpression expr);
DSLPossibleExpression operator=(int expr);

View File

@ -42,6 +42,7 @@ private:
friend DSLType Struct(const char* name, SkTArray<DSLField> fields);
friend class DSLVar;
friend class DSLWriter;
};
} // namespace dsl

View File

@ -64,6 +64,7 @@ private:
friend DSLType Struct(const char* name, SkTArray<DSLField> fields);
friend class DSLFunction;
friend class DSLVar;
friend class DSLWriter;
};
#define TYPE(T) \

View File

@ -10,15 +10,15 @@
#include "include/sksl/DSLExpression.h"
#include "include/sksl/DSLModifiers.h"
#include "include/sksl/DSLType.h"
namespace SkSL {
class Variable;
enum class VariableStorage : int8_t;
namespace dsl {
class DSLType;
class DSLVar {
public:
/**
@ -76,7 +76,7 @@ public:
return DSLExpression(*this).field(name);
}
DSLPossibleExpression operator=(const DSLVar& var) {
DSLPossibleExpression operator=(DSLVar& var) {
return this->operator=(DSLExpression(var));
}
@ -119,12 +119,20 @@ private:
return fName;
}
DSLModifiers fModifiers;
// We only need to keep track of the type here so that we can create the SkSL::Variable. For
// predefined variables this field is unnecessary, so we don't bother tracking it and just set
// it to kVoid; in other words, you shouldn't generally be relying on this field to be correct.
// If you need to determine the variable's type, look at DSLWriter::Var(...).type() instead.
DSLType fType;
int fUniformHandle;
std::unique_ptr<SkSL::Statement> fDeclaration;
bool fDeclared = false;
const SkSL::Variable* fVar;
const SkSL::Variable* fVar = nullptr;
const char* fRawName; // for error reporting
const char* fName;
DSLExpression fInitialValue;
VariableStorage fStorage;
bool fDeclared = false;
friend DSLVar sk_SampleCoord();

View File

@ -78,7 +78,7 @@ public:
DSLWriter::ReportError("error: variable has already been declared\n", &pos);
}
var.fDeclared = true;
return std::move(var.fDeclaration);
return DSLWriter::Declaration(var);
}
static DSLStatement Discard() {

View File

@ -63,10 +63,16 @@ DSLExpression::DSLExpression(bool value)
/*offset=*/-1,
value)) {}
DSLExpression::DSLExpression(const DSLVar& var)
DSLExpression::DSLExpression(DSLVar& var)
: fExpression(std::make_unique<SkSL::VariableReference>(
/*offset=*/-1,
var.fVar,
&DSLWriter::Var(var),
SkSL::VariableReference::RefKind::kRead)) {}
DSLExpression::DSLExpression(DSLVar&& var)
: fExpression(std::make_unique<SkSL::VariableReference>(
/*offset=*/-1,
&DSLWriter::Var(var),
SkSL::VariableReference::RefKind::kRead)) {}
DSLExpression::DSLExpression(DSLPossibleExpression expr, PositionInfo pos) {
@ -252,10 +258,6 @@ DSLExpression DSLPossibleExpression::field(const char* name, PositionInfo pos) {
return DSLExpression(this->release()).field(name, pos);
}
DSLPossibleExpression DSLPossibleExpression::operator=(const DSLVar& var) {
return this->operator=(DSLExpression(var));
}
DSLPossibleExpression DSLPossibleExpression::operator=(DSLExpression expr) {
return DSLExpression(this->release()) = std::move(expr);
}

View File

@ -32,13 +32,13 @@ void DSLFunction::init(const DSLType& returnType, const char* name,
DSLWriter::ReportError("error: using an already-declared variable as a function "
"parameter\n");
}
if (param->fDeclaration && param->fDeclaration->as<VarDeclaration>().value()) {
if (param->fInitialValue.release()) {
DSLWriter::ReportError("error: variables used as function parameters cannot have "
"initial values\n");
}
param->fDeclared = true;
param->fDeclaration = nullptr;
paramVars.push_back(&DSLWriter::Var(*param));
param->fDeclaration = nullptr;
}
SkSL::SymbolTable& symbols = *DSLWriter::SymbolTable();
fDecl = symbols.add(std::make_unique<SkSL::FunctionDeclaration>(

View File

@ -21,10 +21,10 @@ namespace SkSL {
namespace dsl {
DSLVar::DSLVar(const char* name)
: fDeclared(true)
, fVar(nullptr)
: fType(kVoid)
, fRawName(name)
, fName(name) {
, fName(name)
, fDeclared(true) {
#if SK_SUPPORT_GPU && !defined(SKSL_STANDALONE)
if (!strcmp(name, "sk_SampleCoord")) {
fName = DSLWriter::CurrentEmitArgs()->fSampleCoord;
@ -59,13 +59,16 @@ DSLVar::DSLVar(DSLModifiers modifiers, DSLType type, DSLExpression initialValue)
: DSLVar(modifiers, type, "var", std::move(initialValue)) {}
DSLVar::DSLVar(DSLModifiers modifiers, DSLType type, const char* name, DSLExpression initialValue)
: fDeclared(DSLWriter::Instance().fMarkVarsDeclared)
: fModifiers(std::move(modifiers))
, fType(std::move(type))
, fRawName(name)
, fName(DSLWriter::Name(name)) {
Variable::Storage storage = Variable::Storage::kLocal;
, fName(DSLWriter::Name(name))
, fInitialValue(std::move(initialValue))
, fStorage(Variable::Storage::kLocal)
, fDeclared(DSLWriter::Instance().fMarkVarsDeclared) {
#if SK_SUPPORT_GPU && !defined(SKSL_STANDALONE)
if (modifiers.fModifiers.fFlags & Modifiers::kUniform_Flag) {
storage = Variable::Storage::kGlobal;
if (fModifiers.fModifiers.fFlags & Modifiers::kUniform_Flag) {
fStorage = Variable::Storage::kGlobal;
if (DSLWriter::InFragmentProcessor()) {
const SkSL::Type& skslType = type.skslType();
GrSLType grslType;
@ -95,20 +98,6 @@ DSLVar::DSLVar(DSLModifiers modifiers, DSLType type, const char* name, DSLExpres
fDeclared = true;
}
#endif // SK_SUPPORT_GPU && !defined(SKSL_STANDALONE)
DSLWriter::IRGenerator().checkVarDeclaration(/*offset=*/-1, modifiers.fModifiers,
&type.skslType(), storage);
std::unique_ptr<SkSL::Variable> var = DSLWriter::IRGenerator().convertVar(/*offset=*/-1,
modifiers.fModifiers,
&type.skslType(),
fName,
/*isArray=*/false,
/*arraySize=*/nullptr,
storage);
fVar = var.get();
// We can't call VarDeclaration::Convert directly here, because the IRGenerator has special
// treatment for sk_FragColor and sk_RTHeight that we want to preserve in DSL.
fDeclaration = DSLWriter::IRGenerator().convertVarDeclaration(std::move(var),
initialValue.release());
}
DSLVar::~DSLVar() {

View File

@ -196,10 +196,33 @@ void DSLWriter::ReportError(const char* msg, PositionInfo* info) {
}
}
const SkSL::Variable& DSLWriter::Var(const DSLVar& var) {
const SkSL::Variable& DSLWriter::Var(DSLVar& var) {
if (!var.fVar) {
DSLWriter::IRGenerator().checkVarDeclaration(/*offset=*/-1, var.fModifiers.fModifiers,
&var.fType.skslType(), var.fStorage);
std::unique_ptr<SkSL::Variable> skslvar = DSLWriter::IRGenerator().convertVar(
/*offset=*/-1,
var.fModifiers.fModifiers,
&var.fType.skslType(),
var.fName,
/*isArray=*/false,
/*arraySize=*/nullptr,
var.fStorage);
var.fVar = skslvar.get();
// We can't call VarDeclaration::Convert directly here, because the IRGenerator has special
// treatment for sk_FragColor and sk_RTHeight that we want to preserve in DSL.
var.fDeclaration = DSLWriter::IRGenerator().convertVarDeclaration(
std::move(skslvar),
var.fInitialValue.release());
}
return *var.fVar;
}
std::unique_ptr<SkSL::Statement> DSLWriter::Declaration(DSLVar& var) {
Var(var);
return std::move(var.fDeclaration);
}
void DSLWriter::MarkDeclared(DSLVar& var) {
SkASSERT(!var.fDeclared);
var.fDeclared = true;

View File

@ -87,7 +87,12 @@ public:
/**
* Returns the SkSL variable corresponding to a DSLVar.
*/
static const SkSL::Variable& Var(const DSLVar& var);
static const SkSL::Variable& Var(DSLVar& var);
/**
* Returns the SkSL declaration corresponding to a DSLVar.
*/
static std::unique_ptr<SkSL::Statement> Declaration(DSLVar& var);
/**
* For use in testing only: marks the variable as having been declared, so that it can be