SkSL vars now track their declaration instead of their initial value

This should not cause any functional changes, and is just a prerequisite
for upcoming DSL work.

Change-Id: Iea165d3b7ede39ccc9cf5f5d78f623bc883b391e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356816
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
Ethan Nicholas 2021-01-21 13:12:01 -05:00 committed by Skia Commit-Bot
parent ce75036b3e
commit 5b9b0db5b8
10 changed files with 92 additions and 48 deletions

View File

@ -121,6 +121,7 @@ skia_sksl_sources = [
"$_src/sksl/ir/SkSLTypeReference.h",
"$_src/sksl/ir/SkSLUnresolvedFunction.h",
"$_src/sksl/ir/SkSLVarDeclarations.h",
"$_src/sksl/ir/SkSLVariable.cpp",
"$_src/sksl/ir/SkSLVariable.h",
"$_src/sksl/ir/SkSLVariableReference.cpp",
"$_src/sksl/ir/SkSLVariableReference.h",

View File

@ -17,6 +17,7 @@
#include "src/sksl/ir/SkSLFunctionCall.h"
#include "src/sksl/ir/SkSLIfStatement.h"
#include "src/sksl/ir/SkSLIndexExpression.h"
#include "src/sksl/ir/SkSLNop.h"
#include "src/sksl/ir/SkSLPostfixExpression.h"
#include "src/sksl/ir/SkSLPrefixExpression.h"
#include "src/sksl/ir/SkSLReturnStatement.h"
@ -34,12 +35,18 @@ void BasicBlock::Node::setExpression(std::unique_ptr<Expression> expr, ProgramUs
*fExpression = std::move(expr);
}
void BasicBlock::Node::setStatement(std::unique_ptr<Statement> stmt, ProgramUsage* usage) {
std::unique_ptr<Statement> BasicBlock::Node::setStatement(std::unique_ptr<Statement> stmt,
ProgramUsage* usage) {
SkASSERT(!this->isExpression());
// See comment in header - we assume that stmt was already counted in usage (it was a subset
// of fStatement). There is no way to verify that, unfortunately.
usage->remove(fStatement->get());
std::unique_ptr<Statement> result;
if (stmt->is<Nop>()) {
result = std::move(*fStatement);
}
*fStatement = std::move(stmt);
return result;
}
BlockId CFG::newBlock() {

View File

@ -60,7 +60,9 @@ struct BasicBlock {
// or a single Case from a Switch. To maintain usage's bookkeeping, we remove references in
// this node's pointed-to statement. By the time this is called, there is no path from our
// statement to 'stmt', because it's been moved to the argument.
void setStatement(std::unique_ptr<Statement> stmt, ProgramUsage* usage);
// If stmt is Nop, returns the previous statement, otherwise nullptr.
std::unique_ptr<Statement> setStatement(std::unique_ptr<Statement> stmt,
ProgramUsage* usage);
#ifdef SK_DEBUG
String description() const {

View File

@ -1475,7 +1475,12 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
optimizationContext->fNeedsRescan = true;
}
}
(*iter)->setStatement(std::make_unique<Nop>(), usage);
// There can still be (soon to be removed) references to the variable at this point.
// Allowing the VarDeclaration to be destroyed here will break those variable's
// initialValue()s, so we hang on to them until optimization is finished.
std::unique_ptr<Statement> old = (*iter)->setStatement(std::make_unique<Nop>(),
usage);
optimizationContext->fOwnedStatements.push_back(std::move(old));
optimizationContext->fUpdated = true;
}
break;

View File

@ -121,6 +121,8 @@ public:
bool fNeedsRescan = false;
// Metadata about function and variable usage within the program
ProgramUsage* fUsage = nullptr;
// Nodes which we can't throw away until the end of optimization
StatementArray fOwnedStatements;
};
#if !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU

View File

@ -442,20 +442,17 @@ std::unique_ptr<Statement> IRGenerator::convertVarDeclaration(int offset,
if (!value) {
return {};
}
var->setInitialValue(value.get());
}
const Symbol* symbol = (*fSymbolTable)[var->name()];
if (symbol && storage == Variable::Storage::kGlobal && var->name() == "sk_FragColor") {
// Already defined, ignore.
return nullptr;
} else {
std::unique_ptr<Statement> result = std::make_unique<VarDeclaration>(
var.get(),
baseType,
arraySizeValue,
std::move(value));
auto result = std::make_unique<VarDeclaration>(var.get(), baseType, arraySizeValue,
std::move(value));
var->setDeclaration(result.get());
fSymbolTable->add(std::move(var));
return result;
return std::move(result);
}
}
@ -1399,10 +1396,17 @@ void IRGenerator::convertEnum(const ASTNode& e) {
}
value = std::make_unique<IntLiteral>(fContext, e.fOffset, currentValue);
++currentValue;
fSymbolTable->add(std::make_unique<Variable>(e.fOffset, fModifiers->addToPool(modifiers),
child.getString(), type, fIsBuiltinCode,
Variable::Storage::kGlobal, value.get()));
auto var = std::make_unique<Variable>(e.fOffset, fModifiers->addToPool(modifiers),
child.getString(), type, fIsBuiltinCode,
Variable::Storage::kGlobal);
// enum variables aren't really 'declared', but we have to create a declaration to store
// the value
auto declaration = std::make_unique<VarDeclaration>(var.get(), &var->type(),
/*arraySize=*/0, std::move(value));
var->setDeclaration(declaration.get());
fSymbolTable->add(std::move(var));
fSymbolTable->takeOwnershipOfIRNode(std::move(value));
fSymbolTable->takeOwnershipOfIRNode(std::move(declaration));
}
// Now we orphanize the Enum's symbol table, so that future lookups in it are strict
fSymbolTable->fParent = nullptr;

View File

@ -562,19 +562,21 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
auto name = std::make_unique<String>(fMangler.uniqueName(variable.name(),
symbolTableForStatement));
const String* namePtr = symbolTableForStatement->takeOwnershipOfString(std::move(name));
const Variable* clonedVar = symbolTableForStatement->takeOwnershipOfSymbol(
std::make_unique<Variable>(offset,
&variable.modifiers(),
namePtr->c_str(),
variable.type().clone(symbolTableForStatement),
isBuiltinCode,
variable.storage(),
initialValue.get()));
(*varMap)[&variable] = std::make_unique<VariableReference>(offset, clonedVar);
return std::make_unique<VarDeclaration>(clonedVar,
auto clonedVar = std::make_unique<Variable>(
offset,
&variable.modifiers(),
namePtr->c_str(),
variable.type().clone(symbolTableForStatement),
isBuiltinCode,
variable.storage());
(*varMap)[&variable] = std::make_unique<VariableReference>(offset, clonedVar.get());
auto result = std::make_unique<VarDeclaration>(clonedVar.get(),
decl.baseType().clone(symbolTableForStatement),
decl.arraySize(),
std::move(initialValue));
clonedVar->setDeclaration(result.get());
symbolTableForStatement->takeOwnershipOfSymbol(std::move(clonedVar));
return std::move(result);
}
default:
SkASSERT(false);
@ -603,24 +605,24 @@ Inliner::InlineVariable Inliner::makeInlineVariable(const String& baseName,
// Create our new variable and add it to the symbol table.
InlineVariable result;
result.fVarSymbol =
symbolTable->add(std::make_unique<Variable>(/*offset=*/-1,
fModifiers->addToPool(Modifiers()),
nameFrag,
type,
isBuiltinCode,
Variable::Storage::kLocal,
initialValue->get()));
auto var = std::make_unique<Variable>(/*offset=*/-1,
fModifiers->addToPool(Modifiers()),
nameFrag,
type,
isBuiltinCode,
Variable::Storage::kLocal);
// Prepare the variable declaration (taking extra care with `out` params to not clobber any
// initial value).
if (*initialValue && (modifiers.fFlags & Modifiers::kOut_Flag)) {
result.fVarDecl = std::make_unique<VarDeclaration>(result.fVarSymbol, type, /*arraySize=*/0,
result.fVarDecl = std::make_unique<VarDeclaration>(var.get(), type, /*arraySize=*/0,
(*initialValue)->clone());
} else {
result.fVarDecl = std::make_unique<VarDeclaration>(result.fVarSymbol, type, /*arraySize=*/0,
result.fVarDecl = std::make_unique<VarDeclaration>(var.get(), type, /*arraySize=*/0,
std::move(*initialValue));
}
var->setDeclaration(&result.fVarDecl->as<VarDeclaration>());
result.fVarSymbol = symbolTable->add(std::move(var));
return result;
}

View File

@ -289,8 +289,13 @@ std::unique_ptr<ProgramElement> Rehydrator::element() {
SkASSERT(s->kind() == Symbol::Kind::kVariable);
Variable& v = (Variable&) *s;
int value = this->readS32();
v.setInitialValue(symbols->takeOwnershipOfIRNode(
std::make_unique<IntLiteral>(fContext, /*offset=*/-1, value)));
// enum variables aren't really 'declared', but we have to create a declaration to
// store the value
auto valueLiteral = std::make_unique<IntLiteral>(fContext, /*offset=*/-1, value);
auto declaration = std::make_unique<VarDeclaration>(&v, &v.type(), /*arraySize=*/0,
std::move(valueLiteral));
v.setDeclaration(declaration.get());
symbols->takeOwnershipOfIRNode(std::move(declaration));
}
return std::make_unique<Enum>(/*offset=*/-1, typeName, std::move(symbols),
/*isSharedWithCpp=*/true, /*isBuiltin=*/true);
@ -427,10 +432,10 @@ std::unique_ptr<Statement> Rehydrator::statement() {
const Type* baseType = this->type();
int arraySize = this->readS8();
std::unique_ptr<Expression> value = this->expression();
if (value) {
var->setInitialValue(value.get());
}
return std::make_unique<VarDeclaration>(var, baseType, arraySize, std::move(value));
auto result = std::make_unique<VarDeclaration>(var, baseType, arraySize,
std::move(value));
var->setDeclaration(result.get());
return std::move(result);
}
case Rehydrator::kVoid_Command:
return nullptr;

View File

@ -0,0 +1,18 @@
/*
* Copyright 2021 Google LLC.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "src/sksl/ir/SkSLVariable.h"
#include "src/sksl/ir/SkSLVarDeclarations.h"
namespace SkSL {
const Expression* Variable::initialValue() const {
return fDeclaration ? fDeclaration->value().get() : nullptr;
}
} // namespace SkSL

View File

@ -17,6 +17,7 @@
namespace SkSL {
class Expression;
class VarDeclaration;
enum class VariableStorage : int8_t {
kGlobal,
@ -37,9 +38,8 @@ public:
static constexpr Kind kSymbolKind = Kind::kVariable;
Variable(int offset, const Modifiers* modifiers, StringFragment name, const Type* type,
bool builtin, Storage storage, const Expression* initialValue = nullptr)
bool builtin, Storage storage)
: INHERITED(offset, kSymbolKind, name, type)
, fInitialValue(initialValue)
, fModifiers(modifiers)
, fStorage(storage)
, fBuiltin(builtin) {}
@ -56,13 +56,11 @@ public:
return (Storage) fStorage;
}
const Expression* initialValue() const {
return fInitialValue;
}
const Expression* initialValue() const;
void setInitialValue(const Expression* initialValue) {
SkASSERT(!this->initialValue());
fInitialValue = initialValue;
void setDeclaration(VarDeclaration* declaration) {
SkASSERT(!fDeclaration);
fDeclaration = declaration;
}
String description() const override {
@ -70,7 +68,7 @@ public:
}
private:
const Expression* fInitialValue = nullptr;
VarDeclaration* fDeclaration = nullptr;
const Modifiers* fModifiers;
VariableStorage fStorage;
bool fBuiltin;