Simplify VarDeclaration by removing multi-dimensional array support.

Maintaining an array of Expression-based sizes is not necessary as GLSL
only supports a single dimension, and doesn't allow any expression other
than a constant integer or nothing (meaning "unsized").

Change-Id: I01b5f88b94234a27e694aa2fc087f9d5f01b99c5
Bug: skia:11026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340341
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-12-03 10:41:58 -05:00 committed by Skia Commit-Bot
parent 9ea48e3965
commit 62a564686f
10 changed files with 42 additions and 84 deletions

View File

@ -612,11 +612,6 @@ bool TProgramVisitor<PROG, EXPR, STMT, ELEM>::visitStatement(STMT s) {
}
case Statement::Kind::kVarDeclaration: {
auto& v = s.template as<VarDeclaration>();
for (const std::unique_ptr<Expression>& size : v.sizes()) {
if (size && this->visitExpression(*size)) {
return true;
}
}
return v.value() && this->visitExpression(*v.value());
}
case Statement::Kind::kWhile: {

View File

@ -490,10 +490,7 @@ void Dehydrator::write(const Statement* s) {
this->writeCommand(Rehydrator::kVarDeclaration_Command);
this->writeU16(this->symbolId(&v.var()));
this->write(v.baseType());
this->writeU8(v.sizes().count());
for (const std::unique_ptr<Expression>& size : v.sizes()) {
this->write(size.get());
}
this->writeS8(v.arraySize());
this->write(v.value().get());
break;
}

View File

@ -1273,12 +1273,12 @@ void GLSLCodeGenerator::writeVarDeclaration(const VarDeclaration& var, bool glob
this->writeType(var.baseType());
this->write(" ");
this->write(var.var().name());
for (const std::unique_ptr<Expression>& size : var.sizes()) {
if (var.arraySize() > 0) {
this->write("[");
if (size) {
this->writeExpression(*size, kTopLevel_Precedence);
}
this->write(to_string(var.arraySize()));
this->write("]");
} else if (var.arraySize() == Type::kUnsizedArray){
this->write("[]");
}
if (var.value()) {
this->write(" = ");

View File

@ -373,17 +373,16 @@ StatementArray IRGenerator::convertVarDeclarations(const ASTNode& decls,
}
const ASTNode::VarData& varData = varDecl.getVarData();
const Type* type = baseType;
ExpressionArray sizes;
sizes.reserve_back(varData.fSizeCount);
int arraySize = 0;
auto iter = varDecl.begin();
if (iter != varDecl.end()) {
if (type->isOpaque()) {
fErrors.error(type->fOffset,
"opaque type '" + type->name() + "' may not be used in an array");
}
SkSTArray<kMaxArrayDimensionality, int> dimensions;
for (size_t i = 0; i < varData.fSizeCount; ++i, ++iter) {
const ASTNode& rawSize = *iter;
if (varData.fSizeCount > 0) {
SkASSERT(varData.fSizeCount == 1); // only single-dimension arrays are supported
if (type->isOpaque()) {
fErrors.error(type->fOffset,
"opaque type '" + type->name() + "' may not be used in an array");
}
const ASTNode& rawSize = *iter++;
if (rawSize) {
auto size = this->coerce(this->convertExpression(rawSize), *fContext.fInt_Type);
if (!size) {
@ -400,17 +399,12 @@ StatementArray IRGenerator::convertVarDeclarations(const ASTNode& decls,
fErrors.error(size->fOffset, "array size must be positive");
return {};
}
dimensions.push_back(count);
sizes.push_back(std::move(size));
} else if (i == 0) {
dimensions.push_back(Type::kUnsizedArray);
sizes.push_back(nullptr);
arraySize = count;
} else {
fErrors.error(varDecl.fOffset, "array size must be specified");
return {};
arraySize = Type::kUnsizedArray;
}
type = fSymbolTable->addArrayDimensions(type, {arraySize});
}
type = fSymbolTable->addArrayDimensions(type, dimensions);
}
auto var = std::make_unique<Variable>(varDecl.fOffset, fModifiers->addToPool(modifiers),
varData.fName, type, fIsBuiltinCode, storage);
@ -421,7 +415,7 @@ StatementArray IRGenerator::convertVarDeclarations(const ASTNode& decls,
}
std::unique_ptr<Expression> value;
if (iter == varDecl.end()) {
if (varData.fSizeCount > 0 && sizes.front() == nullptr) {
if (arraySize == Type::kUnsizedArray) {
fErrors.error(varDecl.fOffset,
"arrays without an explicit size must use an initializer expression");
return {};
@ -444,8 +438,8 @@ StatementArray IRGenerator::convertVarDeclarations(const ASTNode& decls,
if (symbol && storage == Variable::Storage::kGlobal && var->name() == "sk_FragColor") {
// Already defined, ignore.
} else {
varDecls.push_back(std::make_unique<VarDeclaration>(
var.get(), baseType, std::move(sizes), std::move(value)));
varDecls.push_back(std::make_unique<VarDeclaration>(var.get(), baseType, arraySize,
std::move(value)));
fSymbolTable->add(std::move(var));
}
}
@ -2979,8 +2973,7 @@ IRGenerator::IRBundle IRGenerator::convertProgram(
fContext.fInt_Type.get(), false,
Variable::Storage::kGlobal);
auto decl = std::make_unique<VarDeclaration>(var.get(), fContext.fInt_Type.get(),
/*sizes=*/ExpressionArray{},
/*value=*/nullptr);
/*arraySize=*/0, /*value=*/nullptr);
fSymbolTable->add(std::move(var));
fProgramElements->push_back(
std::make_unique<GlobalVarDeclaration>(/*offset=*/-1, std::move(decl)));

View File

@ -539,12 +539,8 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
}
case Statement::Kind::kVarDeclaration: {
const VarDeclaration& decl = statement.as<VarDeclaration>();
ExpressionArray sizes;
sizes.reserve_back(decl.sizes().count());
for (const std::unique_ptr<Expression>& size : decl.sizes()) {
sizes.push_back(expr(size));
}
std::unique_ptr<Expression> initialValue = expr(decl.value());
int arraySize = decl.arraySize();
const Variable& old = decl.var();
// We assign unique names to inlined variables--scopes hide most of the problems in this
// regard, but see `InlinerAvoidsVariableNameOverlap` for a counterexample where unique
@ -563,7 +559,7 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
old.storage(),
initialValue.get()));
(*varMap)[&old] = std::make_unique<VariableReference>(offset, clone);
return std::make_unique<VarDeclaration>(clone, baseTypePtr, std::move(sizes),
return std::make_unique<VarDeclaration>(clone, baseTypePtr, arraySize,
std::move(initialValue));
}
case Statement::Kind::kWhile: {
@ -643,11 +639,11 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
// initial value).
std::unique_ptr<Statement> variable;
if (initialValue && (modifiers.fFlags & Modifiers::kOut_Flag)) {
variable = std::make_unique<VarDeclaration>(
variableSymbol, type, /*sizes=*/ExpressionArray{}, (*initialValue)->clone());
variable = std::make_unique<VarDeclaration>(variableSymbol, type, /*arraySize=*/0,
(*initialValue)->clone());
} else {
variable = std::make_unique<VarDeclaration>(
variableSymbol, type, /*sizes=*/ExpressionArray{}, std::move(*initialValue));
variable = std::make_unique<VarDeclaration>(variableSymbol, type, /*arraySize=*/0,
std::move(*initialValue));
}
// Add the new variable-declaration statement to our block of extra statements.

View File

@ -1345,12 +1345,12 @@ void MetalCodeGenerator::writeVarDeclaration(const VarDeclaration& var, bool glo
this->disallowArrayTypes(var.baseType()); // `float[2] x` shouldn't be possible (invalid SkSL)
this->write(" ");
this->writeName(var.var().name());
for (const std::unique_ptr<Expression>& size : var.sizes()) {
if (var.arraySize() > 0) {
this->write("[");
if (size) {
this->writeExpression(*size, kTopLevel_Precedence);
}
this->write(to_string(var.arraySize()));
this->write("]");
} else if (var.arraySize() == Type::kUnsizedArray){
this->write("[]");
}
if (var.value()) {
this->write(" = ");

View File

@ -441,18 +441,12 @@ std::unique_ptr<Statement> Rehydrator::statement() {
case Rehydrator::kVarDeclaration_Command: {
Variable* var = this->symbolRef<Variable>(Symbol::Kind::kVariable);
const Type* baseType = this->type();
uint8_t sizeCount = this->readU8();
ExpressionArray sizes;
sizes.reserve_back(sizeCount);
for (int i = 0; i < sizeCount; ++i) {
sizes.push_back(this->expression());
}
int arraySize = this->readS8();
std::unique_ptr<Expression> value = this->expression();
if (value) {
var->setInitialValue(value.get());
}
return std::make_unique<VarDeclaration>(var, baseType, std::move(sizes),
std::move(value));
return std::make_unique<VarDeclaration>(var, baseType, arraySize, std::move(value));
}
case Rehydrator::kVoid_Command:
return nullptr;

View File

@ -352,14 +352,10 @@ static uint8_t SKSL_INCLUDE_sksl_fp[] = {98,1,
50,
49,7,0,
42,6,0,1,
28,
42,6,0,1,0,0,0,
52,
50,
49,9,0,
42,4,0,1,
28,
42,6,0,1,0,0,0,
52,
50,
49,10,0,

View File

@ -68,8 +68,6 @@ static uint8_t SKSL_INCLUDE_sksl_frag[] = {142,0,
50,
49,7,0,
42,6,0,1,
28,
42,6,0,1,0,0,0,
52,
50,
49,8,0,

View File

@ -26,12 +26,12 @@ public:
VarDeclaration(const Variable* var,
const Type* baseType,
ExpressionArray sizes,
int arraySize,
std::unique_ptr<Expression> value)
: INHERITED(var->fOffset, kStatementKind)
, fVar(var)
, fBaseType(*baseType)
, fSizes(std::move(sizes))
, fArraySize(arraySize)
, fValue(std::move(value)) {}
const Type& baseType() const {
@ -46,8 +46,8 @@ public:
fVar = var;
}
const ExpressionArray& sizes() const {
return fSizes;
int arraySize() const {
return fArraySize;
}
std::unique_ptr<Expression>& value() {
@ -59,30 +59,19 @@ public:
}
std::unique_ptr<Statement> clone() const override {
ExpressionArray sizesClone;
sizesClone.reserve_back(this->sizes().count());
for (const std::unique_ptr<Expression>& size : this->sizes()) {
if (size) {
sizesClone.push_back(size->clone());
} else {
sizesClone.push_back(nullptr);
}
}
return std::make_unique<VarDeclaration>(&this->var(),
&this->baseType(),
std::move(sizesClone),
fArraySize,
this->value() ? this->value()->clone() : nullptr);
}
String description() const override {
String result = this->var().modifiers().description() + this->baseType().description() +
" " + this->var().name();
for (const std::unique_ptr<Expression>& size : this->sizes()) {
if (size) {
result += "[" + size->description() + "]";
} else {
result += "[]";
}
if (this->arraySize() > 0) {
result.appendf("[%d]", this->arraySize());
} else if (this->arraySize() == Type::kUnsizedArray){
result += "[]";
}
if (this->value()) {
result += " = " + this->value()->description();
@ -94,7 +83,7 @@ public:
private:
const Variable* fVar;
const Type& fBaseType;
ExpressionArray fSizes;
int fArraySize; // zero means "not an array", Type::kUnsizedArray means var[]
std::unique_ptr<Expression> fValue;
using INHERITED = Statement;