tint: Refactor if-else statement representation

Instead of using an `if` node that has a list of `else` statements,
make each `if` statement have a single optional `else` statement,
which may itself be an `if` statement (or just a block statement).

This better matches the WGSL grammar (now that we have removed
`elseif`), and simplifies various pieces of code that handle these
statements.

Change-Id: Ie4272f1422224490ac598a03aa8b4dd00ba03010
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/87940
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: James Price <jrprice@google.com>
This commit is contained in:
James Price
2022-04-29 00:14:53 +00:00
committed by Dawn LUCI CQ
parent 68e039c456
commit 26ebe5ec36
53 changed files with 392 additions and 940 deletions

View File

@@ -83,14 +83,14 @@ void LoopToForLoop::Run(CloneContext& ctx, const DataMap&, DataMap&) const {
if (!if_stmt) {
return nullptr;
}
auto* else_stmt = tint::As<ast::BlockStatement>(if_stmt->else_statement);
bool negate_condition = false;
if (IsBlockWithSingleBreak(if_stmt->body) &&
if_stmt->else_statements.empty()) {
if_stmt->else_statement == nullptr) {
negate_condition = true;
} else if (if_stmt->body->Empty() && if_stmt->else_statements.size() == 1 &&
if_stmt->else_statements[0]->condition == nullptr &&
IsBlockWithSingleBreak(if_stmt->else_statements[0]->body)) {
} else if (if_stmt->body->Empty() && else_stmt &&
IsBlockWithSingleBreak(else_stmt)) {
negate_condition = false;
} else {
return nullptr;

View File

@@ -302,5 +302,57 @@ fn f() {
EXPECT_EQ(expect, str(got));
}
TEST_F(LoopToForLoopTest, NoTransform_IfBreakWithElse) {
auto* src = R"(
fn f() {
var i : i32;
i = 0;
loop {
if ((i > 15)) {
break;
} else {
}
_ = 123;
continuing {
i = (i + 1);
}
}
}
)";
auto* expect = src;
auto got = Run<LoopToForLoop>(src);
EXPECT_EQ(expect, str(got));
}
TEST_F(LoopToForLoopTest, NoTransform_IfBreakWithElseIf) {
auto* src = R"(
fn f() {
var i : i32;
i = 0;
loop {
if ((i > 15)) {
break;
} else if (true) {
}
_ = 123;
continuing {
i = (i + 1);
}
}
}
)";
auto* expect = src;
auto got = Run<LoopToForLoop>(src);
EXPECT_EQ(expect, str(got));
}
} // namespace
} // namespace tint::transform

View File

@@ -288,7 +288,7 @@ struct MultiplanarExternalTexture::State {
b.Block(
// color = textureLoad(plane0, coord, 0).rgb;
b.Assign("color", b.MemberAccessor(single_plane_call, "rgb"))),
b.Else(b.Block(
b.Block(
// color = vec4<f32>(plane_0_call.r, plane_1_call.rg, 1.0) *
// params.yuvToRgbConversionMatrix;
b.Assign("color",
@@ -296,7 +296,7 @@ struct MultiplanarExternalTexture::State {
b.MemberAccessor(plane_0_call, "r"),
b.MemberAccessor(plane_1_call, "rg"), 1.0f),
b.MemberAccessor(
"params", "yuvToRgbConversionMatrix")))))),
"params", "yuvToRgbConversionMatrix"))))),
// return vec4<f32>(color, 1.0f);
b.Return(b.vec4<f32>("color", 1.0f))};
}

View File

@@ -393,9 +393,6 @@ class DecomposeSideEffects::CollectHoistsState : public StateBase {
[&](const ast::CallStatement* s) { //
ProcessStatement(s->expr);
},
[&](const ast::ElseStatement* s) { //
ProcessStatement(s->condition);
},
[&](const ast::ForLoopStatement* s) {
ProcessStatement(s->condition);
},
@@ -594,18 +591,6 @@ class DecomposeSideEffects::DecomposeState : public StateBase {
InsertBefore(stmts, s);
return ctx.CloneWithoutTransform(s);
},
[&](const ast::ElseStatement* s) -> const ast::Statement* {
if (!s->condition || !sem.Get(s->condition)->HasSideEffects()) {
return nullptr;
}
// NOTE: We shouldn't reach here as else-if with side-effect
// conditions are simplified to else { if } by
// SimplifySideEffectStatements.
ast::StatementList stmts;
ctx.Replace(s->condition, Decompose(s->condition, &stmts));
InsertBefore(stmts, s);
return ctx.CloneWithoutTransform(s);
},
[&](const ast::ForLoopStatement* s) -> const ast::Statement* {
if (!s->condition || !sem.Get(s->condition)->HasSideEffects()) {
return nullptr;

View File

@@ -43,15 +43,6 @@ class State {
Symbol module_discard_var_name; // Use ModuleDiscardVarName() to read
Symbol module_discard_func_name; // Use ModuleDiscardFuncName() to read
// If `block`'s parent is of type TO, returns pointer to it.
template <typename TO>
const TO* ParentAs(const ast::BlockStatement* block) {
if (auto* sem_block = sem.Get(block)) {
return As<TO>(sem_block->Parent());
}
return nullptr;
}
// Returns true if `sem_expr` contains a call expression that may
// (transitively) execute a discard statement.
bool MayDiscard(const sem::Expression* sem_expr) {
@@ -265,14 +256,6 @@ class State {
}
return TryInsertAfter(s, sem_expr);
},
[&](const ast::ElseStatement* s) -> const ast::Statement* {
if (MayDiscard(sem.Get(s->condition))) {
TINT_ICE(Transform, b.Diagnostics())
<< "Unexpected ElseIf condition that may discard. Make sure "
"transform::PromoteSideEffectsToDecl was run first.";
}
return nullptr;
},
[&](const ast::ForLoopStatement* s) -> const ast::Statement* {
if (MayDiscard(sem.Get(s->condition))) {
TINT_ICE(Transform, b.Diagnostics())
@@ -326,20 +309,6 @@ class State {
void Run() {
ctx.ReplaceAll(
[&](const ast::BlockStatement* block) -> const ast::Statement* {
// If this block is for an else-if statement, process the else-if now
// before processing its block statements.
// NOTE: we can't replace else statements at this point - this would
// need to be done when replacing the parent if-statement. However, in
// this transform, we don't ever expect to need to do this as else-ifs
// are converted to else { if } by PromoteSideEffectsToDecl, so this
// is only for validation.
if (auto* sem_else = ParentAs<sem::ElseStatement>(block)) {
if (auto* new_stmt = Statement(sem_else->Declaration())) {
TINT_ASSERT(Transform, new_stmt == nullptr);
return nullptr;
}
}
// Iterate block statements and replace them as needed.
for (auto* stmt : block->statements) {
if (auto* new_stmt = Statement(stmt)) {

View File

@@ -39,25 +39,17 @@ class HoistToDeclBefore::State {
ast::StatementList cont_decls;
};
/// Holds information about 'if's with 'else-if' statements that need to be
/// decomposed into 'if {else}' so that declaration statements can be
/// inserted before the condition expression.
struct IfInfo {
/// Info for each else-if that needs decomposing
struct ElseIfInfo {
/// Decls to insert before condition
ast::StatementList cond_decls;
};
/// 'else if's that need to be decomposed to 'else { if }'
std::unordered_map<const sem::ElseStatement*, ElseIfInfo> else_ifs;
/// Info for each else-if that needs decomposing
struct ElseIfInfo {
/// Decls to insert before condition
ast::StatementList cond_decls;
};
/// For-loops that need to be decomposed to loops.
std::unordered_map<const sem::ForLoopStatement*, LoopInfo> loops;
/// If statements with 'else if's that need to be decomposed to 'else {if}'
std::unordered_map<const sem::IfStatement*, IfInfo> ifs;
/// 'else if' statements that need to be decomposed to 'else {if}'
std::unordered_map<const ast::IfStatement*, ElseIfInfo> else_ifs;
// Converts any for-loops marked for conversion to loops, inserting
// registered declaration statements before the condition or continuing
@@ -118,78 +110,27 @@ class HoistToDeclBefore::State {
}
void ElseIfsToElseWithNestedIfs() {
if (ifs.empty()) {
return;
}
ctx.ReplaceAll([&](const ast::IfStatement* if_stmt) //
-> const ast::IfStatement* {
auto& sem = ctx.src->Sem();
auto* sem_if = sem.Get(if_stmt);
if (!sem_if) {
return nullptr;
}
auto it = ifs.find(sem_if);
if (it == ifs.end()) {
return nullptr;
}
auto& if_info = it->second;
// This if statement has "else if"s that need to be converted to "else
// { if }"s
ast::ElseStatementList next_else_stmts;
next_else_stmts.reserve(if_stmt->else_statements.size());
for (auto* else_stmt : utils::Reverse(if_stmt->else_statements)) {
if (else_stmt->condition == nullptr) {
// The last 'else', keep as is
next_else_stmts.insert(next_else_stmts.begin(), ctx.Clone(else_stmt));
} else {
auto* sem_else_if = sem.Get(else_stmt);
auto it2 = if_info.else_ifs.find(sem_else_if);
if (it2 == if_info.else_ifs.end()) {
// 'else if' we don't need to modify (no decls to insert), so
// keep as is
next_else_stmts.insert(next_else_stmts.begin(),
ctx.Clone(else_stmt));
} else {
// 'else if' we need to replace with 'else <decls> { if }'
auto& else_if_info = it2->second;
// Build the else body's statements, starting with let decls for
// the conditional expression
auto& body_stmts = else_if_info.cond_decls;
// Build nested if
auto* cond = ctx.Clone(else_stmt->condition);
auto* body = ctx.Clone(else_stmt->body);
body_stmts.emplace_back(b.If(cond, body, next_else_stmts));
// Build else
auto* else_with_nested_if = b.Else(b.Block(body_stmts));
// This will be used in parent if (either another nested if, or
// top-level if)
next_else_stmts = {else_with_nested_if};
// Decompose 'else-if' statements into 'else { if }' blocks.
ctx.ReplaceAll(
[&](const ast::IfStatement* else_if) -> const ast::Statement* {
if (!else_ifs.count(else_if)) {
return nullptr;
}
}
}
auto& else_if_info = else_ifs[else_if];
// Build a new top-level if with new else statements
if (next_else_stmts.empty()) {
TINT_ICE(Transform, b.Diagnostics())
<< "Expected else statements to insert into new if";
}
auto* cond = ctx.Clone(if_stmt->condition);
auto* body = ctx.Clone(if_stmt->body);
auto* new_if = b.If(cond, body, next_else_stmts);
return new_if;
});
// Build the else block's body statements, starting with let decls for
// the conditional expression.
auto& body_stmts = else_if_info.cond_decls;
// Move the 'else-if' into the new `else` block as a plain 'if'.
auto* cond = ctx.Clone(else_if->condition);
auto* body = ctx.Clone(else_if->body);
auto* new_if = b.If(cond, body, ctx.Clone(else_if->else_statement));
body_stmts.emplace_back(new_if);
// Replace the 'else-if' with the new 'else' block.
return b.Block(body_stmts);
});
}
public:
@@ -235,13 +176,14 @@ class HoistToDeclBefore::State {
const ast::Statement* stmt) {
auto* ip = before_stmt->Declaration();
if (auto* else_if = before_stmt->As<sem::ElseStatement>()) {
auto* else_if = before_stmt->As<sem::IfStatement>();
if (else_if && else_if->Parent()->Is<sem::IfStatement>()) {
// Insertion point is an 'else if' condition.
// Need to convert 'else if' to 'else { if }'.
auto& if_info = ifs[else_if->Parent()->As<sem::IfStatement>()];
auto& else_if_info = else_ifs[else_if->Declaration()];
// Index the map to convert this else if, even if `stmt` is nullptr.
auto& decls = if_info.else_ifs[else_if].cond_decls;
auto& decls = else_if_info.cond_decls;
if (stmt) {
decls.emplace_back(stmt);
}

View File

@@ -187,8 +187,8 @@ TEST_F(HoistToDeclBeforeTest, ElseIf) {
auto* var = b.Decl(b.Var("a", b.ty.bool_()));
auto* expr = b.Expr("a");
auto* s = b.If(b.Expr(true), b.Block(), //
b.Else(expr, b.Block()), //
b.Else(b.Block()));
b.If(expr, b.Block(), //
b.Block()));
b.Func("f", {}, b.ty.void_(), {var, s});
Program original(std::move(b));
@@ -383,8 +383,8 @@ TEST_F(HoistToDeclBeforeTest, Prepare_ElseIf) {
auto* var = b.Decl(b.Var("a", b.ty.bool_()));
auto* expr = b.Expr("a");
auto* s = b.If(b.Expr(true), b.Block(), //
b.Else(expr, b.Block()), //
b.Else(b.Block()));
b.If(expr, b.Block(), //
b.Block()));
b.Func("f", {}, b.ty.void_(), {var, s});
Program original(std::move(b));
@@ -556,10 +556,9 @@ TEST_F(HoistToDeclBeforeTest, InsertBefore_ElseIf) {
ProgramBuilder b;
b.Func("foo", {}, b.ty.void_(), {});
auto* var = b.Decl(b.Var("a", b.ty.bool_()));
auto* elseif = b.Else(b.Expr("a"), b.Block());
auto* elseif = b.If(b.Expr("a"), b.Block(), b.Block());
auto* s = b.If(b.Expr(true), b.Block(), //
elseif, //
b.Else(b.Block()));
elseif);
b.Func("f", {}, b.ty.void_(), {var, s});
Program original(std::move(b));