AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Migrate DefinitionMap logic to its own class.

Browse Source 
This doesn't change any functionality. It just compartmentalizes a large
chunk of logic that was previously intertwined in the Compiler class.
This logic stands alone and doesn't need anything from the Compiler at
all except the generic "Defined" expression from the Context.

In followup CLs I intend to experiment with changes to the API and
logic, but factoring the code out seemed like a big enough change that
it deserved its own CL.

Change-Id: I3e1a7c62812c6f284167c967086ef4dd828a0b2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367879
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-02-08 17:54:08 -05:00 committed by Skia Commit-Bot
parent 21b8cec137
commit e8a2492b68
9 changed files with 286 additions and 217 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gn/sksl.gni
View File

@ -25,6 +25,8 @@ skia_sksl_sources = [
"$_src/sksl/SkSLContext.cpp",
"$_src/sksl/SkSLContext.h",
"$_src/sksl/SkSLDefines.h",
"$_src/sksl/SkSLDefinitionMap.cpp",
"$_src/sksl/SkSLDefinitionMap.h",
"$_src/sksl/SkSLDehydrator.cpp",
"$_src/sksl/SkSLDehydrator.h",
"$_src/sksl/SkSLErrorReporter.h",

6
src/sksl/SkSLCFGGenerator.cpp
View File

@ -30,14 +30,14 @@
namespace SkSL {
void BasicBlock::Node::setExpression(std::unique_ptr<Expression> expr, ProgramUsage* usage) {
void BasicBlockNode::setExpression(std::unique_ptr<Expression> expr, ProgramUsage* usage) {
SkASSERT(!this->isStatement());
usage->remove(fExpression->get());
*fExpression = std::move(expr);
}
std::unique_ptr<Statement> BasicBlock::Node::setStatement(std::unique_ptr<Statement> stmt,
ProgramUsage* usage) {
std::unique_ptr<Statement> BasicBlockNode::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.

121
src/sksl/SkSLCFGGenerator.h
View File

@ -21,77 +21,82 @@ class ProgramUsage;
// index of a block within CFG.fBlocks
typedef size_t BlockId;
struct BasicBlock {
struct Node {
Node(std::unique_ptr<Statement>* statement)
: fConstantPropagation(false)
, fExpression(nullptr)
, fStatement(statement) {}
// Conceptually, this is part of BasicBlock, but structs inside structs can't be forward-declared;
// this name can be used in places where forward declaration is required.
class BasicBlockNode {
public:
BasicBlockNode(std::unique_ptr<Statement>* statement)
: fConstantPropagation(false)
, fExpression(nullptr)
, fStatement(statement) {}
Node(std::unique_ptr<Expression>* expression, bool constantPropagation)
: fConstantPropagation(constantPropagation)
, fExpression(expression)
, fStatement(nullptr) {}
BasicBlockNode(std::unique_ptr<Expression>* expression, bool constantPropagation)
: fConstantPropagation(constantPropagation)
, fExpression(expression)
, fStatement(nullptr) {}
bool isExpression() const {
return fExpression != nullptr;
}
bool isExpression() const {
return fExpression != nullptr;
}
std::unique_ptr<Expression>* expression() const {
SkASSERT(!this->isStatement());
return fExpression;
}
std::unique_ptr<Expression>* expression() const {
SkASSERT(!this->isStatement());
return fExpression;
}
// See comment below on setStatement. Assumption is that 'expr' is a strict subset of the
// existing expression.
void setExpression(std::unique_ptr<Expression> expr, ProgramUsage* usage);
// See comment below on setStatement. Assumption is that 'expr' is a strict subset of the
// existing expression.
void setExpression(std::unique_ptr<Expression> expr, ProgramUsage* usage);
bool isStatement() const {
return fStatement != nullptr;
}
bool isStatement() const {
return fStatement != nullptr;
}
std::unique_ptr<Statement>* statement() const {
SkASSERT(!this->isExpression());
return fStatement;
}
std::unique_ptr<Statement>* statement() const {
SkASSERT(!this->isExpression());
return fStatement;
}
// Replaces the pointed-to statement with 'stmt'. The assumption is that 'stmt' is a strict
// subset of the existing statement, or a Nop. For example: just the True or False of an if,
// 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.
// If stmt is Nop, returns the previous statement, otherwise nullptr.
std::unique_ptr<Statement> setStatement(std::unique_ptr<Statement> stmt,
ProgramUsage* usage);
// Replaces the pointed-to statement with 'stmt'. The assumption is that 'stmt' is a strict
// subset of the existing statement, or a Nop. For example: just the True or False of an if,
// 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.
// 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 {
SkASSERT(fStatement || fExpression);
if (fStatement) {
return *fStatement ? (*fStatement)->description() : "(null statement)";
} else if (fExpression) {
return *fExpression ? (*fExpression)->description() : "(null expression)";
} else {
return "(nothing)";
}
String description() const {
SkASSERT(fStatement || fExpression);
if (fStatement) {
return *fStatement ? (*fStatement)->description() : "(null statement)";
} else if (fExpression) {
return *fExpression ? (*fExpression)->description() : "(null expression)";
} else {
return "(nothing)";
}
}
#endif
// if false, this node should not be subject to constant propagation. This happens with
// compound assignment (i.e. x *= 2), in which the value x is used as an rvalue for
// multiplication by 2 and then as an lvalue for assignment purposes. Since there is only
// one "x" node, replacing it with a constant would break the assignment and we suppress
// it. Down the road, we should handle this more elegantly by substituting a regular
// assignment if the target is constant (i.e. x = 1; x *= 2; should become x = 1; x = 1 * 2;
// and then collapse down to a simple x = 2;).
bool fConstantPropagation;
// if false, this node should not be subject to constant propagation. This happens with
// compound assignment (i.e. x *= 2), in which the value x is used as an rvalue for
// multiplication by 2 and then as an lvalue for assignment purposes. Since there is only
// one "x" node, replacing it with a constant would break the assignment and we suppress
// it. Down the road, we should handle this more elegantly by substituting a regular
// assignment if the target is constant (i.e. x = 1; x *= 2; should become x = 1; x = 1 * 2;
// and then collapse down to a simple x = 2;).
bool fConstantPropagation;
private:
// we store pointers to the unique_ptrs so that we can replace expressions or statements
// during optimization without having to regenerate the entire tree
std::unique_ptr<Expression>* fExpression;
std::unique_ptr<Statement>* fStatement;
};
private:
// we store pointers to the unique_ptrs so that we can replace expressions or statements
// during optimization without having to regenerate the entire tree
std::unique_ptr<Expression>* fExpression;
std::unique_ptr<Statement>* fStatement;
};
struct BasicBlock {
using Node = BasicBlockNode;
static Node MakeStatement(std::unique_ptr<Statement>* stmt) {
return Node{stmt};

155
src/sksl/SkSLCompiler.cpp
View File

@ -370,141 +370,13 @@ ParsedModule Compiler::parseModule(Program::Kind kind, ModuleData data, const Pa
return {module.fSymbols, std::move(intrinsics)};
}
// add the definition created by assigning to the lvalue to the definition set
void Compiler::addDefinition(const Expression* lvalue, std::unique_ptr<Expression>* expr,
DefinitionMap* definitions) {
switch (lvalue->kind()) {
case Expression::Kind::kVariableReference: {
const Variable& var = *lvalue->as<VariableReference>().variable();
if (var.storage() == Variable::Storage::kLocal) {
definitions->set(&var, expr);
}
break;
}
case Expression::Kind::kSwizzle:
// We consider the variable written to as long as at least some of its components have
// been written to. This will lead to some false negatives (we won't catch it if you
// write to foo.x and then read foo.y), but being stricter could lead to false positives
// (we write to foo.x, and then pass foo to a function which happens to only read foo.x,
// but since we pass foo as a whole it is flagged as an error) unless we perform a much
// more complicated whole-program analysis. This is probably good enough.
this->addDefinition(lvalue->as<Swizzle>().base().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
break;
case Expression::Kind::kIndex:
// see comments in Swizzle
this->addDefinition(lvalue->as<IndexExpression>().base().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
break;
case Expression::Kind::kFieldAccess:
// see comments in Swizzle
this->addDefinition(lvalue->as<FieldAccess>().base().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
break;
case Expression::Kind::kTernary:
// To simplify analysis, we just pretend that we write to both sides of the ternary.
// This allows for false positives (meaning we fail to detect that a variable might not
// have been assigned), but is preferable to false negatives.
this->addDefinition(lvalue->as<TernaryExpression>().ifTrue().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
this->addDefinition(lvalue->as<TernaryExpression>().ifFalse().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
break;
default:
// not an lvalue, can't happen
SkASSERT(false);
}
}
// add local variables defined by this node to the set
void Compiler::addDefinitions(const BasicBlock::Node& node, DefinitionMap* definitions) {
if (node.isExpression()) {
Expression* expr = node.expression()->get();
switch (expr->kind()) {
case Expression::Kind::kBinary: {
BinaryExpression* b = &expr->as<BinaryExpression>();
if (b->getOperator() == Token::Kind::TK_EQ) {
this->addDefinition(b->left().get(), &b->right(), definitions);
} else if (Operators::IsAssignment(b->getOperator())) {
this->addDefinition(
b->left().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
}
break;
}
case Expression::Kind::kFunctionCall: {
const FunctionCall& c = expr->as<FunctionCall>();
const std::vector<const Variable*>& parameters = c.function().parameters();
for (size_t i = 0; i < parameters.size(); ++i) {
if (parameters[i]->modifiers().fFlags & Modifiers::kOut_Flag) {
this->addDefinition(
c.arguments()[i].get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
}
}
break;
}
case Expression::Kind::kPrefix: {
const PrefixExpression* p = &expr->as<PrefixExpression>();
if (p->getOperator() == Token::Kind::TK_MINUSMINUS ||
p->getOperator() == Token::Kind::TK_PLUSPLUS) {
this->addDefinition(
p->operand().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
}
break;
}
case Expression::Kind::kPostfix: {
const PostfixExpression* p = &expr->as<PostfixExpression>();
if (p->getOperator() == Token::Kind::TK_MINUSMINUS ||
p->getOperator() == Token::Kind::TK_PLUSPLUS) {
this->addDefinition(
p->operand().get(),
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
}
break;
}
case Expression::Kind::kVariableReference: {
const VariableReference* v = &expr->as<VariableReference>();
if (v->refKind() != VariableReference::RefKind::kRead) {
this->addDefinition(
v,
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
definitions);
}
break;
}
default:
break;
}
} else if (node.isStatement()) {
Statement* stmt = node.statement()->get();
if (stmt->is<VarDeclaration>()) {
VarDeclaration& vd = stmt->as<VarDeclaration>();
if (vd.value()) {
definitions->set(&vd.var(), &vd.value());
}
}
}
}
void Compiler::scanCFG(CFG* cfg, BlockId blockId, SkBitSet* processedSet) {
BasicBlock& block = cfg->fBlocks[blockId];
// compute definitions after this block
DefinitionMap after = block.fBefore;
for (const BasicBlock::Node& n : block.fNodes) {
this->addDefinitions(n, &after);
after.addDefinitions(*fContext, n);
}
// propagate definitions to exits
@ -518,7 +390,7 @@ void Compiler::scanCFG(CFG* cfg, BlockId blockId, SkBitSet* processedSet) {
if (!exitDef) {
// exit has no definition for it, just copy it and reprocess exit block
processedSet->reset(exitId);
exit.fBefore[var] = e1;
exit.fBefore.set(var, e1);
} else {
// exit has a (possibly different) value already defined
std::unique_ptr<Expression>* e2 = *exitDef;
@ -536,23 +408,6 @@ void Compiler::scanCFG(CFG* cfg, BlockId blockId, SkBitSet* processedSet) {
}
}
// returns a map which maps all local variables in the function to null, indicating that their value
// is initially unknown
static DefinitionMap compute_start_state(const CFG& cfg) {
DefinitionMap result;
for (const auto& block : cfg.fBlocks) {
for (const auto& node : block.fNodes) {
if (node.isStatement()) {
const Statement* s = node.statement()->get();
if (s->is<VarDeclaration>()) {
result[&s->as<VarDeclaration>().var()] = nullptr;
}
}
}
}
return result;
}
/**
* Returns true if assigning to this lvalue has no effect.
*/
@ -593,7 +448,7 @@ static bool dead_assignment(const BinaryExpression& b, ProgramUsage* usage) {
}
void Compiler::computeDataFlow(CFG* cfg) {
cfg->fBlocks[cfg->fStart].fBefore = compute_start_state(*cfg);
cfg->fBlocks[cfg->fStart].fBefore.computeStartState(*cfg);
// We set bits in the "processed" set after a block has been scanned.
SkBitSet processedSet(cfg->fBlocks.size());
@ -859,7 +714,7 @@ void Compiler::simplifyExpression(DefinitionMap& definitions,
const Variable* var = ref.variable();
if (ref.refKind() != VariableReference::RefKind::kWrite &&
ref.refKind() != VariableReference::RefKind::kPointer &&
var->storage() == Variable::Storage::kLocal && !definitions[var] &&
var->storage() == Variable::Storage::kLocal && !definitions.get(var) &&
optimizationContext->fSilences.find(var) == optimizationContext->fSilences.end()) {
optimizationContext->fSilences.insert(var);
this->error(expr->fOffset,
@ -1673,7 +1528,7 @@ bool Compiler::scanCFG(FunctionDefinition& f, ProgramUsage* usage) {
if (optimizationContext.fNeedsRescan) {
break;
}
this->addDefinitions(*iter, &definitions);
definitions.addDefinitions(*fContext, *iter);
}
if (optimizationContext.fNeedsRescan) {

4
src/sksl/SkSLCompiler.h
View File

@ -186,10 +186,6 @@ private:
const ParsedModule& loadPublicModule();
const ParsedModule& loadRuntimeEffectModule();
void addDefinition(const Expression* lvalue, std::unique_ptr<Expression>* expr,
DefinitionMap* definitions);
void addDefinitions(const BasicBlock::Node& node, DefinitionMap* definitions);
void scanCFG(CFG* cfg, BlockId block, SkBitSet* processedSet);
void computeDataFlow(CFG* cfg);

165
src/sksl/SkSLDefinitionMap.cpp Normal file
View File

@ -0,0 +1,165 @@
/*
* 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 <memory>
#include "src/sksl/SkSLDefinitionMap.h"
#include "src/sksl/SkSLCFGGenerator.h"
#include "src/sksl/ir/SkSLBinaryExpression.h"
#include "src/sksl/ir/SkSLExpression.h"
#include "src/sksl/ir/SkSLFieldAccess.h"
#include "src/sksl/ir/SkSLFunctionCall.h"
#include "src/sksl/ir/SkSLIndexExpression.h"
#include "src/sksl/ir/SkSLPostfixExpression.h"
#include "src/sksl/ir/SkSLPrefixExpression.h"
#include "src/sksl/ir/SkSLSwizzle.h"
#include "src/sksl/ir/SkSLTernaryExpression.h"
namespace SkSL {
/**
* Maps all local variables in the function to null, indicating that their value is initially
* unknown.
*/
void DefinitionMap::computeStartState(const CFG& cfg) {
fDefinitions.reset();
for (const BasicBlock& block : cfg.fBlocks) {
for (const BasicBlock::Node& node : block.fNodes) {
if (node.isStatement()) {
const Statement* s = node.statement()->get();
if (s->is<VarDeclaration>()) {
fDefinitions[&s->as<VarDeclaration>().var()] = nullptr;
}
}
}
}
}
// Add the definition created by assigning to the lvalue to the definition map.
void DefinitionMap::addDefinition(const Context& context, const Expression* lvalue,
std::unique_ptr<Expression>* expr) {
switch (lvalue->kind()) {
case Expression::Kind::kVariableReference: {
const Variable& var = *lvalue->as<VariableReference>().variable();
if (var.storage() == Variable::Storage::kLocal) {
fDefinitions.set(&var, expr);
}
break;
}
case Expression::Kind::kSwizzle:
// We consider the variable written to as long as at least some of its components have
// been written to. This will lead to some false negatives (we won't catch it if you
// write to foo.x and then read foo.y), but being stricter could lead to false positives
// (we write to foo.x, and then pass foo to a function which happens to only read foo.x,
// but since we pass foo as a whole it is flagged as an error) unless we perform a much
// more complicated whole-program analysis. This is probably good enough.
this->addDefinition(context, lvalue->as<Swizzle>().base().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
break;
case Expression::Kind::kIndex:
// see comments in Swizzle
this->addDefinition(context, lvalue->as<IndexExpression>().base().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
break;
case Expression::Kind::kFieldAccess:
// see comments in Swizzle
this->addDefinition(context, lvalue->as<FieldAccess>().base().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
break;
case Expression::Kind::kTernary:
// To simplify analysis, we just pretend that we write to both sides of the ternary.
// This allows for false positives (meaning we fail to detect that a variable might not
// have been assigned), but is preferable to false negatives.
this->addDefinition(context, lvalue->as<TernaryExpression>().ifTrue().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
this->addDefinition(context, lvalue->as<TernaryExpression>().ifFalse().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
break;
default:
SkDEBUGFAILF("expression is not an lvalue: %s", lvalue->description().c_str());
break;
}
}
// Add local variables defined by this node to the map.
void DefinitionMap::addDefinitions(const Context& context, const BasicBlock::Node& node) {
if (node.isExpression()) {
Expression* expr = node.expression()->get();
switch (expr->kind()) {
case Expression::Kind::kBinary: {
BinaryExpression* b = &expr->as<BinaryExpression>();
if (b->getOperator() == Token::Kind::TK_EQ) {
this->addDefinition(context, b->left().get(), &b->right());
} else if (Operators::IsAssignment(b->getOperator())) {
this->addDefinition(
context, b->left().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
}
break;
}
case Expression::Kind::kFunctionCall: {
const FunctionCall& c = expr->as<FunctionCall>();
const std::vector<const Variable*>& parameters = c.function().parameters();
for (size_t i = 0; i < parameters.size(); ++i) {
if (parameters[i]->modifiers().fFlags & Modifiers::kOut_Flag) {
this->addDefinition(
context, c.arguments()[i].get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
}
}
break;
}
case Expression::Kind::kPrefix: {
const PrefixExpression* p = &expr->as<PrefixExpression>();
if (p->getOperator() == Token::Kind::TK_MINUSMINUS ||
p->getOperator() == Token::Kind::TK_PLUSPLUS) {
this->addDefinition(
context, p->operand().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
}
break;
}
case Expression::Kind::kPostfix: {
const PostfixExpression* p = &expr->as<PostfixExpression>();
if (p->getOperator() == Token::Kind::TK_MINUSMINUS ||
p->getOperator() == Token::Kind::TK_PLUSPLUS) {
this->addDefinition(
context, p->operand().get(),
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
}
break;
}
case Expression::Kind::kVariableReference: {
const VariableReference* v = &expr->as<VariableReference>();
if (v->refKind() != VariableReference::RefKind::kRead) {
this->addDefinition(
context, v,
(std::unique_ptr<Expression>*)&context.fDefined_Expression);
}
break;
}
default:
break;
}
} else if (node.isStatement()) {
Statement* stmt = node.statement()->get();
if (stmt->is<VarDeclaration>()) {
VarDeclaration& vd = stmt->as<VarDeclaration>();
if (vd.value()) {
fDefinitions.set(&vd.var(), &vd.value());
}
}
}
}
} // namespace SkSL

46
src/sksl/SkSLDefinitionMap.h Normal file
View File

@ -0,0 +1,46 @@
/*
* Copyright 2021 Google LLC
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#ifndef SKSL_DEFINITION_MAP
#define SKSL_DEFINITION_MAP
#include <memory>
#include "include/private/SkTHash.h"
namespace SkSL {
class BasicBlockNode;
struct CFG;
class Context;
class Expression;
class Variable;
class DefinitionMap {
public:
using MapType = SkTHashMap<const Variable*, std::unique_ptr<Expression>*>;
// Allow callers to set, search and iterate directly, like a THashMap.
void set(const Variable* v, std::unique_ptr<Expression>* e) { fDefinitions.set(v, e); }
std::unique_ptr<Expression>** find(const Variable* v) const { return fDefinitions.find(v); }
std::unique_ptr<Expression>* get(const Variable* v) { return fDefinitions[v]; }
MapType::Iter begin() const { return fDefinitions.begin(); }
MapType::Iter end() const { return fDefinitions.end(); }
// These methods populate the definition map from the CFG.
void computeStartState(const CFG& cfg);
void addDefinition(const Context& context, const Expression* lvalue,
std::unique_ptr<Expression>* expr);
void addDefinitions(const Context& context, const BasicBlockNode& node);
private:
SkTHashMap<const Variable*, std::unique_ptr<Expression>*> fDefinitions;
};
} // namespace SkSL
#endif

3
src/sksl/ir/SkSLExpression.h
View File

@ -9,6 +9,7 @@
#define SKSL_EXPRESSION
#include "include/private/SkTHash.h"
#include "src/sksl/SkSLDefinitionMap.h"
#include "src/sksl/ir/SkSLStatement.h"
#include "src/sksl/ir/SkSLType.h"
@ -20,8 +21,6 @@ class Expression;
class IRGenerator;
class Variable;
using DefinitionMap = SkTHashMap<const Variable*, std::unique_ptr<Expression>*>;
/**
* Abstract supertype of all expressions.
*/

1
src/sksl/ir/SkSLVariableReference.cpp
View File

@ -7,6 +7,7 @@
#include "src/sksl/ir/SkSLVariableReference.h"
#include "src/sksl/SkSLDefinitionMap.h"
#include "src/sksl/SkSLIRGenerator.h"
#include "src/sksl/ir/SkSLConstructor.h"
#include "src/sksl/ir/SkSLFloatLiteral.h"