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

Commit 0d31ed5

Browse files
Ethan NicholasSkia Commit-Bot
Ethan Nicholas
authored and
Skia Commit-Bot
committed
moved SkSL ForStatement data into IRNode
Change-Id: I87039ae982c7f5b6ed7a4cc236470f049606c45e Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321468 Commit-Queue: Ethan Nicholas <[email protected]> Reviewed-by: John Stiles <[email protected]>
1 parent ae4bb98 commit 0d31ed5

13 files changed

+154
-87
lines changed

src/sksl/SkSLAnalysis.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,10 @@ bool TProgramVisitor<PROG, EXPR, STMT, ELEM>::visitStatement(STMT s) {
445445

446446
case Statement::Kind::kFor: {
447447
auto& f = s.template as<ForStatement>();
448-
return (f.fInitializer && this->visitStatement(*f.fInitializer)) ||
449-
(f.fTest && this->visitExpression(*f.fTest)) ||
450-
(f.fNext && this->visitExpression(*f.fNext)) ||
451-
this->visitStatement(*f.fStatement);
448+
return (f.initializer() && this->visitStatement(*f.initializer())) ||
449+
(f.test() && this->visitExpression(*f.test())) ||
450+
(f.next() && this->visitExpression(*f.next())) ||
451+
this->visitStatement(*f.statement());
452452
}
453453
case Statement::Kind::kIf: {
454454
auto& i = s.template as<IfStatement>();

src/sksl/SkSLByteCodeGenerator.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -1703,21 +1703,21 @@ void ByteCodeGenerator::writeDoStatement(const DoStatement& d) {
17031703
void ByteCodeGenerator::writeForStatement(const ForStatement& f) {
17041704
fContinueTargets.emplace();
17051705
fBreakTargets.emplace();
1706-
if (f.fInitializer) {
1707-
this->writeStatement(*f.fInitializer);
1706+
if (f.initializer()) {
1707+
this->writeStatement(*f.initializer());
17081708
}
17091709
this->write(ByteCodeInstruction::kLoopBegin);
17101710
size_t start = fCode->size();
1711-
if (f.fTest) {
1712-
this->writeExpression(*f.fTest);
1711+
if (f.test()) {
1712+
this->writeExpression(*f.test());
17131713
this->write(ByteCodeInstruction::kLoopMask);
17141714
}
17151715
this->write(ByteCodeInstruction::kBranchIfAllFalse);
17161716
DeferredLocation endLocation(this);
1717-
this->writeStatement(*f.fStatement);
1717+
this->writeStatement(*f.statement());
17181718
this->write(ByteCodeInstruction::kLoopNext);
1719-
if (f.fNext) {
1720-
this->writeExpression(*f.fNext, true);
1719+
if (f.next()) {
1720+
this->writeExpression(*f.next(), true);
17211721
}
17221722
this->write(ByteCodeInstruction::kBranch);
17231723
this->write16(start);

src/sksl/SkSLCFGGenerator.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -582,16 +582,16 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
582582
}
583583
case Statement::Kind::kFor: {
584584
ForStatement& f = (*s)->as<ForStatement>();
585-
if (f.fInitializer) {
586-
this->addStatement(cfg, &f.fInitializer);
585+
if (f.initializer()) {
586+
this->addStatement(cfg, &f.initializer());
587587
}
588588
BlockId loopStart = cfg.newBlock();
589589
BlockId next = cfg.newIsolatedBlock();
590590
fLoopContinues.push(next);
591591
BlockId loopExit = cfg.newIsolatedBlock();
592592
fLoopExits.push(loopExit);
593-
if (f.fTest) {
594-
this->addExpression(cfg, &f.fTest, /*constantPropagate=*/true);
593+
if (f.test()) {
594+
this->addExpression(cfg, &f.test(), /*constantPropagate=*/true);
595595
// this isn't quite right; we should have an exit from here to the loop exit, and
596596
// remove the exit from the loop body to the loop exit. Structuring it like this
597597
// forces the optimizer to believe that the loop body is always executed at least
@@ -601,11 +601,11 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
601601
// guaranteed to happen, but for the time being we take the easy way out.
602602
}
603603
cfg.newBlock();
604-
this->addStatement(cfg, &f.fStatement);
604+
this->addStatement(cfg, &f.statement());
605605
cfg.addExit(cfg.fCurrent, next);
606606
cfg.fCurrent = next;
607-
if (f.fNext) {
608-
this->addExpression(cfg, &f.fNext, /*constantPropagate=*/true);
607+
if (f.next()) {
608+
this->addExpression(cfg, &f.next(), /*constantPropagate=*/true);
609609
}
610610
cfg.addExit(cfg.fCurrent, loopStart);
611611
cfg.addExit(cfg.fCurrent, loopExit);

src/sksl/SkSLDehydrator.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -426,11 +426,11 @@ void Dehydrator::write(const Statement* s) {
426426
case Statement::Kind::kFor: {
427427
const ForStatement& f = s->as<ForStatement>();
428428
this->writeU8(Rehydrator::kFor_Command);
429-
this->write(f.fInitializer.get());
430-
this->write(f.fTest.get());
431-
this->write(f.fNext.get());
432-
this->write(f.fStatement.get());
433-
this->write(f.fSymbols);
429+
this->write(f.initializer().get());
430+
this->write(f.test().get());
431+
this->write(f.next().get());
432+
this->write(f.statement().get());
433+
this->write(f.symbols());
434434
break;
435435
}
436436
case Statement::Kind::kIf: {

src/sksl/SkSLGLSLCodeGenerator.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -1370,28 +1370,28 @@ void GLSLCodeGenerator::writeIfStatement(const IfStatement& stmt) {
13701370

13711371
void GLSLCodeGenerator::writeForStatement(const ForStatement& f) {
13721372
this->write("for (");
1373-
if (f.fInitializer && !f.fInitializer->isEmpty()) {
1374-
this->writeStatement(*f.fInitializer);
1373+
if (f.initializer() && !f.initializer()->isEmpty()) {
1374+
this->writeStatement(*f.initializer());
13751375
} else {
13761376
this->write("; ");
13771377
}
1378-
if (f.fTest) {
1378+
if (f.test()) {
13791379
if (fProgram.fSettings.fCaps->addAndTrueToLoopCondition()) {
13801380
std::unique_ptr<Expression> and_true(new BinaryExpression(
1381-
-1, f.fTest->clone(), Token::Kind::TK_LOGICALAND,
1381+
-1, f.test()->clone(), Token::Kind::TK_LOGICALAND,
13821382
std::make_unique<BoolLiteral>(fContext, -1, true),
13831383
fContext.fBool_Type.get()));
13841384
this->writeExpression(*and_true, kTopLevel_Precedence);
13851385
} else {
1386-
this->writeExpression(*f.fTest, kTopLevel_Precedence);
1386+
this->writeExpression(*f.test(), kTopLevel_Precedence);
13871387
}
13881388
}
13891389
this->write("; ");
1390-
if (f.fNext) {
1391-
this->writeExpression(*f.fNext, kTopLevel_Precedence);
1390+
if (f.next()) {
1391+
this->writeExpression(*f.next(), kTopLevel_Precedence);
13921392
}
13931393
this->write(") ");
1394-
this->writeStatement(*f.fStatement);
1394+
this->writeStatement(*f.statement());
13951395
}
13961396

13971397
void GLSLCodeGenerator::writeWhileStatement(const WhileStatement& w) {

src/sksl/SkSLIRGenerator.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ std::unique_ptr<Statement> IRGenerator::convertFor(const ASTNode& f) {
587587
auto forStmt = std::make_unique<ForStatement>(f.fOffset, std::move(initializer),
588588
std::move(test), std::move(next),
589589
std::move(statement), fSymbolTable);
590-
fInliner->ensureScopedBlocks(forStmt->fStatement.get(), forStmt.get());
590+
fInliner->ensureScopedBlocks(forStmt->statement().get(), forStmt.get());
591591
return std::move(forStmt);
592592
}
593593

src/sksl/SkSLInliner.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -483,9 +483,9 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
483483
const ForStatement& f = statement.as<ForStatement>();
484484
// need to ensure initializer is evaluated first so that we've already remapped its
485485
// declarations by the time we evaluate test & next
486-
std::unique_ptr<Statement> initializer = stmt(f.fInitializer);
487-
return std::make_unique<ForStatement>(offset, std::move(initializer), expr(f.fTest),
488-
expr(f.fNext), stmt(f.fStatement), f.fSymbols);
486+
std::unique_ptr<Statement> initializer = stmt(f.initializer());
487+
return std::make_unique<ForStatement>(offset, std::move(initializer), expr(f.test()),
488+
expr(f.next()), stmt(f.statement()), f.symbols());
489489
}
490490
case Statement::Kind::kIf: {
491491
const IfStatement& i = statement.as<IfStatement>();
@@ -884,14 +884,14 @@ class InlineCandidateAnalyzer {
884884
}
885885
case Statement::Kind::kFor: {
886886
ForStatement& forStmt = (*stmt)->as<ForStatement>();
887-
if (forStmt.fSymbols) {
888-
fSymbolTableStack.push_back(forStmt.fSymbols.get());
887+
if (forStmt.symbols()) {
888+
fSymbolTableStack.push_back(forStmt.symbols().get());
889889
}
890890

891891
// The initializer and loop body are candidates for inlining.
892-
this->visitStatement(&forStmt.fInitializer,
892+
this->visitStatement(&forStmt.initializer(),
893893
/*isViableAsEnclosingStatement=*/false);
894-
this->visitStatement(&forStmt.fStatement);
894+
this->visitStatement(&forStmt.statement());
895895

896896
// The inliner isn't smart enough to inline the test- or increment-expressions
897897
// of a for loop loop at this time. There are a handful of limitations:

src/sksl/SkSLMetalCodeGenerator.cpp

+11-11
Original file line numberDiff line numberDiff line change
@@ -1332,20 +1332,20 @@ void MetalCodeGenerator::writeIfStatement(const IfStatement& stmt) {
13321332

13331333
void MetalCodeGenerator::writeForStatement(const ForStatement& f) {
13341334
this->write("for (");
1335-
if (f.fInitializer && !f.fInitializer->isEmpty()) {
1336-
this->writeStatement(*f.fInitializer);
1335+
if (f.initializer() && !f.initializer()->isEmpty()) {
1336+
this->writeStatement(*f.initializer());
13371337
} else {
13381338
this->write("; ");
13391339
}
1340-
if (f.fTest) {
1341-
this->writeExpression(*f.fTest, kTopLevel_Precedence);
1340+
if (f.test()) {
1341+
this->writeExpression(*f.test(), kTopLevel_Precedence);
13421342
}
13431343
this->write("; ");
1344-
if (f.fNext) {
1345-
this->writeExpression(*f.fNext, kTopLevel_Precedence);
1344+
if (f.next()) {
1345+
this->writeExpression(*f.next(), kTopLevel_Precedence);
13461346
}
13471347
this->write(") ");
1348-
this->writeStatement(*f.fStatement);
1348+
this->writeStatement(*f.statement());
13491349
}
13501350

13511351
void MetalCodeGenerator::writeWhileStatement(const WhileStatement& w) {
@@ -1816,10 +1816,10 @@ MetalCodeGenerator::Requirements MetalCodeGenerator::requirements(const Statemen
18161816
}
18171817
case Statement::Kind::kFor: {
18181818
const ForStatement& f = s->as<ForStatement>();
1819-
return this->requirements(f.fInitializer.get()) |
1820-
this->requirements(f.fTest.get()) |
1821-
this->requirements(f.fNext.get()) |
1822-
this->requirements(f.fStatement.get());
1819+
return this->requirements(f.initializer().get()) |
1820+
this->requirements(f.test().get()) |
1821+
this->requirements(f.next().get()) |
1822+
this->requirements(f.statement().get());
18231823
}
18241824
case Statement::Kind::kWhile: {
18251825
const WhileStatement& w = s->as<WhileStatement>();

src/sksl/SkSLSPIRVCodeGenerator.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -2952,8 +2952,8 @@ void SPIRVCodeGenerator::writeIfStatement(const IfStatement& stmt, OutputStream&
29522952
}
29532953

29542954
void SPIRVCodeGenerator::writeForStatement(const ForStatement& f, OutputStream& out) {
2955-
if (f.fInitializer) {
2956-
this->writeStatement(*f.fInitializer, out);
2955+
if (f.initializer()) {
2956+
this->writeStatement(*f.initializer(), out);
29572957
}
29582958
SpvId header = this->nextId();
29592959
SpvId start = this->nextId();
@@ -2967,18 +2967,18 @@ void SPIRVCodeGenerator::writeForStatement(const ForStatement& f, OutputStream&
29672967
this->writeInstruction(SpvOpLoopMerge, end, next, SpvLoopControlMaskNone, out);
29682968
this->writeInstruction(SpvOpBranch, start, out);
29692969
this->writeLabel(start, out);
2970-
if (f.fTest) {
2971-
SpvId test = this->writeExpression(*f.fTest, out);
2970+
if (f.test()) {
2971+
SpvId test = this->writeExpression(*f.test(), out);
29722972
this->writeInstruction(SpvOpBranchConditional, test, body, end, out);
29732973
}
29742974
this->writeLabel(body, out);
2975-
this->writeStatement(*f.fStatement, out);
2975+
this->writeStatement(*f.statement(), out);
29762976
if (fCurrentBlock) {
29772977
this->writeInstruction(SpvOpBranch, next, out);
29782978
}
29792979
this->writeLabel(next, out);
2980-
if (f.fNext) {
2981-
this->writeExpression(*f.fNext, out);
2980+
if (f.next()) {
2981+
this->writeExpression(*f.next(), out);
29822982
}
29832983
this->writeInstruction(SpvOpBranch, header, out);
29842984
this->writeLabel(end, out);

src/sksl/ir/SkSLForStatement.h

+61-28
Original file line numberDiff line numberDiff line change
@@ -17,55 +17,88 @@ namespace SkSL {
1717
/**
1818
* A 'for' statement.
1919
*/
20-
struct ForStatement : public Statement {
20+
class ForStatement : public Statement {
21+
public:
2122
static constexpr Kind kStatementKind = Kind::kFor;
2223

2324
ForStatement(int offset, std::unique_ptr<Statement> initializer,
2425
std::unique_ptr<Expression> test, std::unique_ptr<Expression> next,
2526
std::unique_ptr<Statement> statement, std::shared_ptr<SymbolTable> symbols)
26-
: INHERITED(offset, kStatementKind)
27-
, fSymbols(symbols)
28-
, fInitializer(std::move(initializer))
29-
, fTest(std::move(test))
30-
, fNext(std::move(next))
31-
, fStatement(std::move(statement)) {}
27+
: INHERITED(offset, kStatementKind, ForStatementData{std::move(symbols)}) {
28+
fStatementChildren.reserve(2);
29+
fStatementChildren.push_back(std::move(initializer));
30+
fStatementChildren.push_back(std::move(statement));
31+
fExpressionChildren.reserve(2);
32+
fExpressionChildren.push_back(std::move(test));
33+
fExpressionChildren.push_back(std::move(next));
34+
}
35+
36+
std::unique_ptr<Statement>& initializer() {
37+
return fStatementChildren[0];
38+
}
39+
40+
const std::unique_ptr<Statement>& initializer() const {
41+
return fStatementChildren[0];
42+
}
43+
44+
std::unique_ptr<Expression>& test() {
45+
return fExpressionChildren[0];
46+
}
47+
48+
const std::unique_ptr<Expression>& test() const {
49+
return fExpressionChildren[0];
50+
}
51+
52+
std::unique_ptr<Expression>& next() {
53+
return fExpressionChildren[1];
54+
}
55+
56+
const std::unique_ptr<Expression>& next() const {
57+
return fExpressionChildren[1];
58+
}
59+
60+
std::unique_ptr<Statement>& statement() {
61+
return fStatementChildren[1];
62+
}
63+
64+
const std::unique_ptr<Statement>& statement() const {
65+
return fStatementChildren[1];
66+
}
67+
68+
std::shared_ptr<SymbolTable> symbols() const {
69+
return this->forStatementData().fSymbolTable;
70+
}
3271

3372
std::unique_ptr<Statement> clone() const override {
34-
return std::unique_ptr<Statement>(new ForStatement(fOffset,
35-
fInitializer ? fInitializer->clone() : nullptr,
36-
fTest ? fTest->clone() : nullptr,
37-
fNext ? fNext->clone() : nullptr,
38-
fStatement->clone(),
39-
fSymbols));
73+
return std::unique_ptr<Statement>(new ForStatement(
74+
fOffset,
75+
this->initializer() ? this->initializer()->clone() : nullptr,
76+
this->test() ? this->test()->clone() : nullptr,
77+
this->next() ? this->next()->clone() : nullptr,
78+
this->statement()->clone(),
79+
this->symbols()));
4080
}
4181

4282
String description() const override {
4383
String result("for (");
44-
if (fInitializer) {
45-
result += fInitializer->description();
84+
if (this->initializer()) {
85+
result += this->initializer()->description();
4686
} else {
4787
result += ";";
4888
}
4989
result += " ";
50-
if (fTest) {
51-
result += fTest->description();
90+
if (this->test()) {
91+
result += this->test()->description();
5292
}
5393
result += "; ";
54-
if (fNext) {
55-
result += fNext->description();
94+
if (this->next()) {
95+
result += this->next()->description();
5696
}
57-
result += ") " + fStatement->description();
97+
result += ") " + this->statement()->description();
5898
return result;
5999
}
60100

61-
// it's important to keep fSymbols defined first (and thus destroyed last) because destroying
62-
// the other fields can update symbol reference counts
63-
const std::shared_ptr<SymbolTable> fSymbols;
64-
std::unique_ptr<Statement> fInitializer;
65-
std::unique_ptr<Expression> fTest;
66-
std::unique_ptr<Expression> fNext;
67-
std::unique_ptr<Statement> fStatement;
68-
101+
private:
69102
using INHERITED = Statement;
70103
};
71104

src/sksl/ir/SkSLIRNode.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ IRNode::IRNode(int offset, int kind, const FloatLiteralData& data)
4848
, fKind(kind)
4949
, fData(data) {}
5050

51+
IRNode::IRNode(int offset, int kind, const ForStatementData& data)
52+
: fOffset(offset)
53+
, fKind(kind)
54+
, fData(data) {}
55+
5156
IRNode::IRNode(int offset, int kind, const String& data)
5257
: fOffset(offset)
5358
, fKind(kind)

0 commit comments

Comments
 (0)