Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 453f67f

Browse files
Ethan NicholasSkia Commit-Bot
Ethan Nicholas
authored and
Skia Commit-Bot
committed
SkSL enum changes
Changed a couple of SkSL enums to enum classes and rearranged things to make their storage within IRNode type safe. Change-Id: I6509d027d79161c1a09473e90943aae061583f20 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/324624 Reviewed-by: John Stiles <johnstiles@google.com> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
1 parent e70a30d commit 453f67f

14 files changed

+93
-82
lines changed

‎src/sksl/SkSLAnalysis.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,10 @@ class VariableWriteVisitor : public ProgramVisitor {
193193
bool visitExpression(const Expression& e) override {
194194
if (e.is<VariableReference>()) {
195195
const VariableReference& ref = e.as<VariableReference>();
196-
if (ref.variable() == fVar && (ref.refKind() == VariableReference::kWrite_RefKind ||
197-
ref.refKind() == VariableReference::kReadWrite_RefKind ||
198-
ref.refKind() == VariableReference::kPointer_RefKind)) {
196+
if (ref.variable() == fVar &&
197+
(ref.refKind() == VariableReference::RefKind::kWrite ||
198+
ref.refKind() == VariableReference::RefKind::kReadWrite ||
199+
ref.refKind() == VariableReference::RefKind::kPointer)) {
199200
return true;
200201
}
201202
}

‎src/sksl/SkSLByteCodeGenerator.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ std::unique_ptr<ByteCodeFunction> ByteCodeGenerator::writeFunction(const Functio
218218
static int expression_as_builtin(const Expression& e) {
219219
if (e.is<VariableReference>()) {
220220
const Variable& var(*e.as<VariableReference>().variable());
221-
if (var.storage() == Variable::kGlobal_Storage) {
221+
if (var.storage() == Variable::Storage::kGlobal) {
222222
return var.modifiers().fLayout.fBuiltin;
223223
}
224224
}
@@ -424,7 +424,7 @@ ByteCodeGenerator::Location ByteCodeGenerator::getLocation(const Variable& var)
424424
// given that we seldom have more than a couple of variables, linear search is probably the most
425425
// efficient way to handle lookups
426426
switch (var.storage()) {
427-
case Variable::kLocal_Storage: {
427+
case Variable::Storage::kLocal: {
428428
for (int i = fLocals.size() - 1; i >= 0; --i) {
429429
if (fLocals[i] == &var) {
430430
SkASSERT(fParameterCount + i <= 255);
@@ -439,7 +439,7 @@ ByteCodeGenerator::Location ByteCodeGenerator::getLocation(const Variable& var)
439439
SkASSERT(result <= 255);
440440
return { result, Storage::kLocal };
441441
}
442-
case Variable::kParameter_Storage: {
442+
case Variable::Storage::kParameter: {
443443
int offset = 0;
444444
for (const auto& p : fFunction->fDeclaration.parameters()) {
445445
if (p == &var) {
@@ -451,7 +451,7 @@ ByteCodeGenerator::Location ByteCodeGenerator::getLocation(const Variable& var)
451451
SkASSERT(false);
452452
return Location::MakeInvalid();
453453
}
454-
case Variable::kGlobal_Storage: {
454+
case Variable::Storage::kGlobal: {
455455
if (var.type() == *fContext.fFragmentProcessor_Type) {
456456
int offset = 0;
457457
for (const auto& e : fProgram.elements()) {

‎src/sksl/SkSLCPPCodeGenerator.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ static bool is_private(const Variable& var) {
138138
const Modifiers& modifiers = var.modifiers();
139139
return !(modifiers.fFlags & Modifiers::kUniform_Flag) &&
140140
!(modifiers.fFlags & Modifiers::kIn_Flag) &&
141-
var.storage() == Variable::kGlobal_Storage &&
141+
var.storage() == Variable::Storage::kGlobal &&
142142
modifiers.fLayout.fBuiltin == -1;
143143
}
144144

‎src/sksl/SkSLCompiler.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,10 @@ Compiler::Compiler(Flags flags)
207207
StringFragment skCapsName("sk_Caps");
208208
fRootSymbolTable->add(std::make_unique<Variable>(/*offset=*/-1,
209209
fIRGenerator->fModifiers->handle(Modifiers()),
210-
skCapsName, fContext->fSkCaps_Type.get(),
211-
/*builtin=*/false, Variable::kGlobal_Storage));
210+
skCapsName,
211+
fContext->fSkCaps_Type.get(),
212+
/*builtin=*/false,
213+
Variable::Storage::kGlobal));
212214

213215
fRootModule = {fRootSymbolTable, /*fIntrinsics=*/nullptr};
214216

@@ -350,7 +352,7 @@ void Compiler::addDefinition(const Expression* lvalue, std::unique_ptr<Expressio
350352
switch (lvalue->kind()) {
351353
case Expression::Kind::kVariableReference: {
352354
const Variable& var = *lvalue->as<VariableReference>().variable();
353-
if (var.storage() == Variable::kLocal_Storage) {
355+
if (var.storage() == Variable::Storage::kLocal) {
354356
definitions->set(&var, expr);
355357
}
356358
break;
@@ -452,7 +454,7 @@ void Compiler::addDefinitions(const BasicBlock::Node& node, DefinitionMap* defin
452454
}
453455
case Expression::Kind::kVariableReference: {
454456
const VariableReference* v = &expr->as<VariableReference>();
455-
if (v->refKind() != VariableReference::kRead_RefKind) {
457+
if (v->refKind() != VariableReference::RefKind::kRead) {
456458
this->addDefinition(
457459
v,
458460
(std::unique_ptr<Expression>*) &fContext->fDefined_Expression,
@@ -785,7 +787,7 @@ static void vectorize_right(BasicBlock* b,
785787
static void clear_write(Expression& expr) {
786788
switch (expr.kind()) {
787789
case Expression::Kind::kVariableReference: {
788-
expr.as<VariableReference>().setRefKind(VariableReference::kRead_RefKind);
790+
expr.as<VariableReference>().setRefKind(VariableReference::RefKind::kRead);
789791
break;
790792
}
791793
case Expression::Kind::kFieldAccess:
@@ -829,9 +831,9 @@ void Compiler::simplifyExpression(DefinitionMap& definitions,
829831
case Expression::Kind::kVariableReference: {
830832
const VariableReference& ref = expr->as<VariableReference>();
831833
const Variable* var = ref.variable();
832-
if (ref.refKind() != VariableReference::kWrite_RefKind &&
833-
ref.refKind() != VariableReference::kPointer_RefKind &&
834-
var->storage() == Variable::kLocal_Storage && !definitions[var] &&
834+
if (ref.refKind() != VariableReference::RefKind::kWrite &&
835+
ref.refKind() != VariableReference::RefKind::kPointer &&
836+
var->storage() == Variable::Storage::kLocal && !definitions[var] &&
835837
(*undefinedVariables).find(var) == (*undefinedVariables).end()) {
836838
(*undefinedVariables).insert(var);
837839
this->error(expr->fOffset,

‎src/sksl/SkSLDehydrator.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ void Dehydrator::write(const Symbol& s) {
220220
this->write(v.modifiers());
221221
this->write(v.name());
222222
this->write(v.type());
223-
this->writeU8(v.storage());
223+
this->writeU8((int8_t) v.storage());
224224
break;
225225
}
226226
case Symbol::Kind::kField: {
@@ -379,7 +379,7 @@ void Dehydrator::write(const Expression* e) {
379379
const VariableReference& v = e->as<VariableReference>();
380380
this->writeCommand(Rehydrator::kVariableReference_Command);
381381
this->writeId(v.variable());
382-
this->writeU8(v.refKind());
382+
this->writeU8((int8_t) v.refKind());
383383
break;
384384
}
385385
case Expression::Kind::kFunctionReference:

‎src/sksl/SkSLIRGenerator.cpp‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ std::unique_ptr<Block> IRGenerator::convertBlock(const ASTNode& block) {
296296

297297
std::unique_ptr<Statement> IRGenerator::convertVarDeclarationStatement(const ASTNode& s) {
298298
SkASSERT(s.fKind == ASTNode::Kind::kVarDeclarations);
299-
auto decls = this->convertVarDeclarations(s, Variable::kLocal_Storage);
299+
auto decls = this->convertVarDeclarations(s, Variable::Storage::kLocal);
300300
if (decls.empty()) {
301301
return nullptr;
302302
}
@@ -320,7 +320,7 @@ std::vector<std::unique_ptr<Statement>> IRGenerator::convertVarDeclarations(
320320
return {};
321321
}
322322
if (baseType->nonnullable() == *fContext.fFragmentProcessor_Type &&
323-
storage != Variable::kGlobal_Storage) {
323+
storage != Variable::Storage::kGlobal) {
324324
fErrors.error(decls.fOffset,
325325
"variables of type '" + baseType->displayName() + "' must be global");
326326
}
@@ -397,7 +397,7 @@ std::vector<std::unique_ptr<Statement>> IRGenerator::convertVarDeclarations(
397397
}
398398
}
399399
int permitted = Modifiers::kConst_Flag;
400-
if (storage == Variable::kGlobal_Storage) {
400+
if (storage == Variable::Storage::kGlobal) {
401401
permitted |= Modifiers::kIn_Flag | Modifiers::kOut_Flag | Modifiers::kUniform_Flag |
402402
Modifiers::kFlat_Flag | Modifiers::kVarying_Flag |
403403
Modifiers::kNoPerspective_Flag | Modifiers::kPLS_Flag |
@@ -468,9 +468,9 @@ std::vector<std::unique_ptr<Statement>> IRGenerator::convertVarDeclarations(
468468
var->setInitialValue(value.get());
469469
}
470470
Symbol* symbol = (*fSymbolTable)[var->name()];
471-
if (symbol && storage == Variable::kGlobal_Storage && var->name() == "sk_FragColor") {
471+
if (symbol && storage == Variable::Storage::kGlobal && var->name() == "sk_FragColor") {
472472
// Already defined, ignore.
473-
} else if (symbol && storage == Variable::kGlobal_Storage &&
473+
} else if (symbol && storage == Variable::Storage::kGlobal &&
474474
symbol->kind() == Symbol::Kind::kVariable &&
475475
symbol->as<Variable>().modifiers().fLayout.fBuiltin >= 0) {
476476
// Already defined, just update the modifiers.
@@ -789,9 +789,9 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
789789
fContext.fBool_Type.get()));
790790
std::unique_ptr<Expression> next(new PostfixExpression(
791791
std::unique_ptr<Expression>(
792-
new VariableReference(-1,
793-
loopIdx,
794-
VariableReference::kReadWrite_RefKind)),
792+
new VariableReference(-1,
793+
loopIdx,
794+
VariableReference::RefKind::kReadWrite)),
795795
Token::Kind::TK_PLUSPLUS));
796796
ASTNode endPrimitiveID(&fFile->fNodes, -1, ASTNode::Kind::kIdentifier, "EndPrimitive");
797797
std::unique_ptr<Expression> endPrimitive = this->convertExpression(endPrimitiveID);
@@ -809,7 +809,7 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
809809
std::vector<std::unique_ptr<Expression>>()))));
810810
std::unique_ptr<Expression> assignment(new BinaryExpression(-1,
811811
std::unique_ptr<Expression>(new VariableReference(-1, loopIdx,
812-
VariableReference::kWrite_RefKind)),
812+
VariableReference::RefKind::kWrite)),
813813
Token::Kind::TK_EQ,
814814
std::make_unique<IntLiteral>(fContext, -1, 0),
815815
fContext.fInt_Type.get()));
@@ -832,9 +832,9 @@ std::unique_ptr<Statement> IRGenerator::getNormalizeSkPositionCode() {
832832
// sk_Position.w);
833833
SkASSERT(fSkPerVertex && fRTAdjust);
834834
#define REF(var) std::unique_ptr<Expression>(\
835-
new VariableReference(-1, var, VariableReference::kRead_RefKind))
835+
new VariableReference(-1, var, VariableReference::RefKind::kRead))
836836
#define WREF(var) std::unique_ptr<Expression>(\
837-
new VariableReference(-1, var, VariableReference::kWrite_RefKind))
837+
new VariableReference(-1, var, VariableReference::RefKind::kWrite))
838838
#define FIELD(var, idx) std::unique_ptr<Expression>(\
839839
new FieldAccess(REF(var), idx, FieldAccess::kAnonymousInterfaceBlock_OwnerKind))
840840
#define POS std::unique_ptr<Expression>(new FieldAccess(WREF(fSkPerVertex), 0, \
@@ -964,7 +964,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
964964
Variable* var = fSymbolTable->takeOwnershipOfSymbol(
965965
std::make_unique<Variable>(param.fOffset, fModifiers->handle(pd.fModifiers),
966966
name, type, fIsBuiltinCode,
967-
Variable::kParameter_Storage));
967+
Variable::Storage::kParameter));
968968
parameters.push_back(var);
969969
}
970970

@@ -1137,7 +1137,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTNode
11371137
auto iter = intf.begin();
11381138
for (size_t i = 0; i < id.fDeclarationCount; ++i) {
11391139
std::vector<std::unique_ptr<Statement>> decls =
1140-
this->convertVarDeclarations(*(iter++), Variable::kInterfaceBlock_Storage);
1140+
this->convertVarDeclarations(*(iter++), Variable::Storage::kInterfaceBlock);
11411141
if (decls.empty()) {
11421142
return nullptr;
11431143
}
@@ -1205,7 +1205,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTNode
12051205
id.fInstanceName.fLength ? id.fInstanceName : id.fTypeName,
12061206
type,
12071207
fIsBuiltinCode,
1208-
Variable::kGlobal_Storage));
1208+
Variable::Storage::kGlobal));
12091209
if (foundRTAdjust) {
12101210
fRTAdjustInterfaceBlock = var;
12111211
}
@@ -1274,7 +1274,7 @@ void IRGenerator::convertEnum(const ASTNode& e) {
12741274
++currentValue;
12751275
fSymbolTable->add(std::make_unique<Variable>(e.fOffset, fModifiers->handle(modifiers),
12761276
child.getString(), type, fIsBuiltinCode,
1277-
Variable::kGlobal_Storage, value.get()));
1277+
Variable::Storage::kGlobal, value.get()));
12781278
fSymbolTable->takeOwnershipOfIRNode(std::move(value));
12791279
}
12801280
// Now we orphanize the Enum's symbol table, so that future lookups in it are strict
@@ -1431,12 +1431,12 @@ std::unique_ptr<Expression> IRGenerator::convertIdentifier(const ASTNode& identi
14311431
// default to kRead_RefKind; this will be corrected later if the variable is written to
14321432
return std::make_unique<VariableReference>(identifier.fOffset,
14331433
var,
1434-
VariableReference::kRead_RefKind);
1434+
VariableReference::RefKind::kRead);
14351435
}
14361436
case Symbol::Kind::kField: {
14371437
const Field* field = &result->as<Field>();
14381438
auto base = std::make_unique<VariableReference>(identifier.fOffset, &field->owner(),
1439-
VariableReference::kRead_RefKind);
1439+
VariableReference::RefKind::kRead);
14401440
return std::make_unique<FieldAccess>(std::move(base),
14411441
field->fieldIndex(),
14421442
FieldAccess::kAnonymousInterfaceBlock_OwnerKind);
@@ -1967,8 +1967,8 @@ std::unique_ptr<Expression> IRGenerator::convertBinaryExpression(const ASTNode&
19671967
}
19681968
if (Compiler::IsAssignment(op)) {
19691969
if (!this->setRefKind(*left, op != Token::Kind::TK_EQ
1970-
? VariableReference::kReadWrite_RefKind
1971-
: VariableReference::kWrite_RefKind)) {
1970+
? VariableReference::RefKind::kReadWrite
1971+
: VariableReference::RefKind::kWrite)) {
19721972
return nullptr;
19731973
}
19741974
}
@@ -2122,8 +2122,8 @@ std::unique_ptr<Expression> IRGenerator::call(int offset,
21222122
const Modifiers& paramModifiers = function.parameters()[i]->modifiers();
21232123
if (paramModifiers.fFlags & Modifiers::kOut_Flag) {
21242124
if (!this->setRefKind(*arguments[i], paramModifiers.fFlags & Modifiers::kIn_Flag
2125-
? VariableReference::kReadWrite_RefKind
2126-
: VariableReference::kPointer_RefKind)) {
2125+
? VariableReference::RefKind::kReadWrite
2126+
: VariableReference::RefKind::kPointer)) {
21272127
return nullptr;
21282128
}
21292129
}
@@ -2407,7 +2407,7 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
24072407
"' cannot operate on '" + baseType.displayName() + "'");
24082408
return nullptr;
24092409
}
2410-
if (!this->setRefKind(*base, VariableReference::kReadWrite_RefKind)) {
2410+
if (!this->setRefKind(*base, VariableReference::RefKind::kReadWrite)) {
24112411
return nullptr;
24122412
}
24132413
break;
@@ -2418,7 +2418,7 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
24182418
"' cannot operate on '" + baseType.displayName() + "'");
24192419
return nullptr;
24202420
}
2421-
if (!this->setRefKind(*base, VariableReference::kReadWrite_RefKind)) {
2421+
if (!this->setRefKind(*base, VariableReference::RefKind::kReadWrite)) {
24222422
return nullptr;
24232423
}
24242424
break;
@@ -2832,7 +2832,7 @@ std::unique_ptr<Expression> IRGenerator::convertPostfixExpression(const ASTNode&
28322832
"' cannot operate on '" + baseType.displayName() + "'");
28332833
return nullptr;
28342834
}
2835-
if (!this->setRefKind(*base, VariableReference::kReadWrite_RefKind)) {
2835+
if (!this->setRefKind(*base, VariableReference::RefKind::kReadWrite)) {
28362836
return nullptr;
28372837
}
28382838
return std::make_unique<PostfixExpression>(std::move(base), expression.getToken().fKind);
@@ -2975,7 +2975,7 @@ void IRGenerator::convertProgram(Program::Kind kind,
29752975
switch (decl.fKind) {
29762976
case ASTNode::Kind::kVarDeclarations: {
29772977
std::vector<std::unique_ptr<Statement>> decls =
2978-
this->convertVarDeclarations(decl, Variable::kGlobal_Storage);
2978+
this->convertVarDeclarations(decl, Variable::Storage::kGlobal);
29792979
for (auto& varDecl : decls) {
29802980
fProgramElements->push_back(std::make_unique<GlobalVarDeclaration>(
29812981
decl.fOffset, std::move(varDecl)));

0 commit comments

Comments
 (0)