Skip to content

Commit d6f2338

Browse files
brianosmanSkia Commit-Bot
brianosman
authored and
Skia Commit-Bot
committed
SkSL IR normalization: Convert while loops to for loops
Bug: skia:11095 Change-Id: Icd69df40675e5ecde5004e04a7dcd78eedf8343c Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344765 Commit-Queue: Brian Osman <[email protected]> Reviewed-by: John Stiles <[email protected]> Reviewed-by: Ethan Nicholas <[email protected]> Reviewed-by: Mike Klein <[email protected]>
1 parent ba158b9 commit d6f2338

21 files changed

+24
-222
lines changed

‎gn/sksl.gni‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,6 @@ skia_sksl_sources = [
116116
"$_src/sksl/ir/SkSLVariable.h",
117117
"$_src/sksl/ir/SkSLVariableReference.cpp",
118118
"$_src/sksl/ir/SkSLVariableReference.h",
119-
"$_src/sksl/ir/SkSLWhileStatement.h",
120119
]
121120

122121
skia_sksl_gpu_sources = [

‎src/sksl/SkSLAnalysis.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
#include "src/sksl/ir/SkSLNop.h"
3636
#include "src/sksl/ir/SkSLReturnStatement.h"
3737
#include "src/sksl/ir/SkSLSwitchStatement.h"
38-
#include "src/sksl/ir/SkSLWhileStatement.h"
3938

4039
// Expressions
4140
#include "src/sksl/ir/SkSLBinaryExpression.h"
@@ -616,10 +615,6 @@ bool TProgramVisitor<PROG, EXPR, STMT, ELEM>::visitStatement(STMT s) {
616615
auto& v = s.template as<VarDeclaration>();
617616
return v.value() && this->visitExpression(*v.value());
618617
}
619-
case Statement::Kind::kWhile: {
620-
auto& w = s.template as<WhileStatement>();
621-
return this->visitExpression(*w.test()) || this->visitStatement(*w.statement());
622-
}
623618
default:
624619
SkUNREACHABLE;
625620
}

‎src/sksl/SkSLByteCodeGenerator.cpp‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,21 +1912,6 @@ void ByteCodeGenerator::writeVarDeclaration(const VarDeclaration& decl) {
19121912
}
19131913
}
19141914

1915-
void ByteCodeGenerator::writeWhileStatement(const WhileStatement& w) {
1916-
this->write(ByteCodeInstruction::kLoopBegin);
1917-
size_t cond = fCode->size();
1918-
this->writeExpression(*w.test());
1919-
this->write(ByteCodeInstruction::kLoopMask);
1920-
this->write(ByteCodeInstruction::kBranchIfAllFalse);
1921-
DeferredLocation endLocation(this);
1922-
this->writeStatement(*w.statement());
1923-
this->write(ByteCodeInstruction::kLoopNext);
1924-
this->write(ByteCodeInstruction::kBranch);
1925-
this->write16(cond);
1926-
endLocation.set();
1927-
this->write(ByteCodeInstruction::kLoopEnd);
1928-
}
1929-
19301915
void ByteCodeGenerator::writeStatement(const Statement& s) {
19311916
switch (s.kind()) {
19321917
case Statement::Kind::kBlock:
@@ -1962,9 +1947,6 @@ void ByteCodeGenerator::writeStatement(const Statement& s) {
19621947
case Statement::Kind::kVarDeclaration:
19631948
this->writeVarDeclaration(s.as<VarDeclaration>());
19641949
break;
1965-
case Statement::Kind::kWhile:
1966-
this->writeWhileStatement(s.as<WhileStatement>());
1967-
break;
19681950
case Statement::Kind::kInlineMarker:
19691951
case Statement::Kind::kNop:
19701952
break;

‎src/sksl/SkSLByteCodeGenerator.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
#include "src/sksl/ir/SkSLTernaryExpression.h"
4646
#include "src/sksl/ir/SkSLVarDeclarations.h"
4747
#include "src/sksl/ir/SkSLVariableReference.h"
48-
#include "src/sksl/ir/SkSLWhileStatement.h"
4948
#include "src/sksl/spirv.h"
5049

5150
namespace SkSL {
@@ -282,7 +281,6 @@ class ByteCodeGenerator : public CodeGenerator {
282281
void writeContinueStatement(const ContinueStatement& c);
283282
void writeIfStatement(const IfStatement& stmt);
284283
void writeForStatement(const ForStatement& f);
285-
void writeWhileStatement(const WhileStatement& w);
286284
void writeDoStatement(const DoStatement& d);
287285
void writeSwitchStatement(const SwitchStatement& s);
288286
void writeReturnStatement(const ReturnStatement& r);

‎src/sksl/SkSLCFGGenerator.cpp‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "src/sksl/ir/SkSLSwitchStatement.h"
2424
#include "src/sksl/ir/SkSLSwizzle.h"
2525
#include "src/sksl/ir/SkSLTernaryExpression.h"
26-
#include "src/sksl/ir/SkSLWhileStatement.h"
2726

2827
namespace SkSL {
2928

@@ -487,10 +486,6 @@ void CFGGenerator::addLValue(CFG& cfg, std::unique_ptr<Expression>* e) {
487486
}
488487
}
489488

490-
static bool is_true(Expression& expr) {
491-
return expr.is<BoolLiteral>() && expr.as<BoolLiteral>().value();
492-
}
493-
494489
void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
495490
switch ((*s)->kind()) {
496491
case Statement::Kind::kBlock: {
@@ -556,25 +551,6 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
556551
cfg.addExit(cfg.fCurrent, fLoopContinues.top());
557552
cfg.fCurrent = cfg.newIsolatedBlock();
558553
break;
559-
case Statement::Kind::kWhile: {
560-
WhileStatement& w = (*s)->as<WhileStatement>();
561-
BlockId loopStart = cfg.newBlock();
562-
fLoopContinues.push(loopStart);
563-
BlockId loopExit = cfg.newIsolatedBlock();
564-
fLoopExits.push(loopExit);
565-
this->addExpression(cfg, &w.test(), /*constantPropagate=*/true);
566-
BlockId test = cfg.fCurrent;
567-
if (!is_true(*w.test())) {
568-
cfg.addExit(test, loopExit);
569-
}
570-
cfg.newBlock();
571-
this->addStatement(cfg, &w.statement());
572-
cfg.addExit(cfg.fCurrent, loopStart);
573-
fLoopContinues.pop();
574-
fLoopExits.pop();
575-
cfg.fCurrent = loopExit;
576-
break;
577-
}
578554
case Statement::Kind::kDo: {
579555
DoStatement& d = (*s)->as<DoStatement>();
580556
BlockId loopStart = cfg.newBlock();

‎src/sksl/SkSLDehydrator.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
#include "src/sksl/ir/SkSLUnresolvedFunction.h"
4848
#include "src/sksl/ir/SkSLVarDeclarations.h"
4949
#include "src/sksl/ir/SkSLVariable.h"
50-
#include "src/sksl/ir/SkSLWhileStatement.h"
5150

5251
#ifdef SKSL_STANDALONE
5352

@@ -494,13 +493,6 @@ void Dehydrator::write(const Statement* s) {
494493
this->write(v.value().get());
495494
break;
496495
}
497-
case Statement::Kind::kWhile: {
498-
const WhileStatement& w = s->as<WhileStatement>();
499-
this->writeCommand(Rehydrator::kWhile_Command);
500-
this->write(w.test().get());
501-
this->write(w.statement().get());
502-
break;
503-
}
504496
}
505497
} else {
506498
this->writeCommand(Rehydrator::kVoid_Command);

‎src/sksl/SkSLGLSLCodeGenerator.cpp‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,9 +1308,6 @@ void GLSLCodeGenerator::writeStatement(const Statement& s) {
13081308
case Statement::Kind::kFor:
13091309
this->writeForStatement(s.as<ForStatement>());
13101310
break;
1311-
case Statement::Kind::kWhile:
1312-
this->writeWhileStatement(s.as<WhileStatement>());
1313-
break;
13141311
case Statement::Kind::kDo:
13151312
this->writeDoStatement(s.as<DoStatement>());
13161313
break;
@@ -1368,6 +1365,15 @@ void GLSLCodeGenerator::writeIfStatement(const IfStatement& stmt) {
13681365
}
13691366

13701367
void GLSLCodeGenerator::writeForStatement(const ForStatement& f) {
1368+
// Emit loops of the form 'for(;test;)' as 'while(test)', which is probably how they started
1369+
if (!f.initializer() && f.test() && !f.next()) {
1370+
this->write("while (");
1371+
this->writeExpression(*f.test(), kTopLevel_Precedence);
1372+
this->write(") ");
1373+
this->writeStatement(*f.statement());
1374+
return;
1375+
}
1376+
13711377
this->write("for (");
13721378
if (f.initializer() && !f.initializer()->isEmpty()) {
13731379
this->writeStatement(*f.initializer());
@@ -1393,13 +1399,6 @@ void GLSLCodeGenerator::writeForStatement(const ForStatement& f) {
13931399
this->writeStatement(*f.statement());
13941400
}
13951401

1396-
void GLSLCodeGenerator::writeWhileStatement(const WhileStatement& w) {
1397-
this->write("while (");
1398-
this->writeExpression(*w.test(), kTopLevel_Precedence);
1399-
this->write(") ");
1400-
this->writeStatement(*w.statement());
1401-
}
1402-
14031402
void GLSLCodeGenerator::writeDoStatement(const DoStatement& d) {
14041403
if (!fProgram.fCaps->rewriteDoWhileLoops()) {
14051404
this->write("do ");

‎src/sksl/SkSLGLSLCodeGenerator.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
#include "src/sksl/ir/SkSLTernaryExpression.h"
4242
#include "src/sksl/ir/SkSLVarDeclarations.h"
4343
#include "src/sksl/ir/SkSLVariableReference.h"
44-
#include "src/sksl/ir/SkSLWhileStatement.h"
4544

4645
namespace SkSL {
4746

@@ -189,8 +188,6 @@ class GLSLCodeGenerator : public CodeGenerator {
189188

190189
void writeForStatement(const ForStatement& f);
191190

192-
void writeWhileStatement(const WhileStatement& w);
193-
194191
void writeDoStatement(const DoStatement& d);
195192

196193
virtual void writeSwitchStatement(const SwitchStatement& s);

‎src/sksl/SkSLIRGenerator.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
#include "src/sksl/ir/SkSLVarDeclarations.h"
5858
#include "src/sksl/ir/SkSLVariable.h"
5959
#include "src/sksl/ir/SkSLVariableReference.h"
60-
#include "src/sksl/ir/SkSLWhileStatement.h"
6160

6261
namespace SkSL {
6362

@@ -559,7 +558,8 @@ std::unique_ptr<Statement> IRGenerator::convertWhile(const ASTNode& w) {
559558
if (!statement) {
560559
return nullptr;
561560
}
562-
return std::make_unique<WhileStatement>(w.fOffset, std::move(test), std::move(statement));
561+
return std::make_unique<ForStatement>(w.fOffset, /*initializer=*/nullptr, std::move(test),
562+
/*next=*/nullptr, std::move(statement), fSymbolTable);
563563
}
564564

565565
std::unique_ptr<Statement> IRGenerator::convertDo(const ASTNode& d) {

‎src/sksl/SkSLInliner.cpp‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
#include "src/sksl/ir/SkSLVarDeclarations.h"
5252
#include "src/sksl/ir/SkSLVariable.h"
5353
#include "src/sksl/ir/SkSLVariableReference.h"
54-
#include "src/sksl/ir/SkSLWhileStatement.h"
5554

5655
namespace SkSL {
5756
namespace {
@@ -100,7 +99,6 @@ static int count_returns_at_end_of_control_flow(const FunctionDefinition& funcDe
10099
this->visitStatement(*block.children().back());
101100
}
102101
case Statement::Kind::kSwitch:
103-
case Statement::Kind::kWhile:
104102
case Statement::Kind::kDo:
105103
case Statement::Kind::kFor:
106104
// Don't introspect switches or loop structures at all.
@@ -132,7 +130,6 @@ static int count_returns_in_breakable_constructs(const FunctionDefinition& funcD
132130
bool visitStatement(const Statement& stmt) override {
133131
switch (stmt.kind()) {
134132
case Statement::Kind::kSwitch:
135-
case Statement::Kind::kWhile:
136133
case Statement::Kind::kDo:
137134
case Statement::Kind::kFor: {
138135
++fInsideBreakableConstruct;
@@ -257,7 +254,7 @@ void Inliner::ensureScopedBlocks(Statement* inlinedBody, Statement* parentStmt)
257254

258255
// No changes necessary if the parent statement doesn't require a scope.
259256
if (!parentStmt || !(parentStmt->is<IfStatement>() || parentStmt->is<ForStatement>() ||
260-
parentStmt->is<DoStatement>() || parentStmt->is<WhileStatement>())) {
257+
parentStmt->is<DoStatement>())) {
261258
return;
262259
}
263260

@@ -574,10 +571,6 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
574571
return std::make_unique<VarDeclaration>(clone, baseTypePtr, arraySize,
575572
std::move(initialValue));
576573
}
577-
case Statement::Kind::kWhile: {
578-
const WhileStatement& w = statement.as<WhileStatement>();
579-
return std::make_unique<WhileStatement>(offset, expr(w.test()), stmt(w.statement()));
580-
}
581574
default:
582575
SkASSERT(false);
583576
return nullptr;
@@ -956,20 +949,6 @@ class InlineCandidateAnalyzer {
956949
this->visitExpression(&varDeclStmt.value());
957950
break;
958951
}
959-
case Statement::Kind::kWhile: {
960-
WhileStatement& whileStmt = (*stmt)->as<WhileStatement>();
961-
// The loop body is a candidate for inlining.
962-
this->visitStatement(&whileStmt.statement());
963-
// The inliner isn't smart enough to inline the test-expression for a while loop at
964-
// this time. There are two limitations:
965-
// - We would need to insert the inlined-body block at the very beginning of the
966-
// while loop's inner fStatement. We don't support that today, but it's doable.
967-
// - The while-loop's built-in test-expression would need to be replaced with a
968-
// `true` BoolLiteral, and the loop would be halted via a break statement at the
969-
// end of the inlined test-expression. This is again something we don't support
970-
// today, but it could be implemented.
971-
break;
972-
}
973952
default:
974953
SkUNREACHABLE;
975954
}

0 commit comments

Comments
 (0)