[ir][validation] Walk through if/switch/loop in the validator
Add support for walking through the blocks of if/switch and loop now
that the structure has settled. Fixup a few tests along the way that are
broken due to failed validation.
This updates the disassembler to assume that an if always has a true
block and a loop always has a body. This is then validated by the generic block code that a terminator is required in a block.
Bug: tint:1952
Change-Id: I8e18e8b314b1b113602fa279143b96d870706690
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/139505
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/ir/disassembler.cc b/src/tint/ir/disassembler.cc
index 38e9be4..5bb2201 100644
--- a/src/tint/ir/disassembler.cc
+++ b/src/tint/ir/disassembler.cc
@@ -538,18 +538,11 @@
out_ << "if ";
EmitOperand(if_, if_->Condition(), If::kConditionOperandOffset);
- bool has_true = !if_->True()->IsEmpty();
bool has_false = !if_->False()->IsEmpty();
- out_ << " [";
- if (has_true) {
- out_ << "t: %b" << IdOf(if_->True());
- }
+ out_ << " [t: %b" << IdOf(if_->True());
if (has_false) {
- if (has_true) {
- out_ << ", ";
- }
- out_ << "f: %b" << IdOf(if_->False());
+ out_ << ", f: %b" << IdOf(if_->False());
}
out_ << "]";
sm.Store(if_);
@@ -557,10 +550,12 @@
out_ << " { # " << NameOf(if_);
EmitLine();
- if (has_true) {
+ // True block is assumed to have instructions
+ {
ScopedIndent si(indent_size_);
EmitBlock(if_->True(), "true");
}
+
if (has_false) {
ScopedIndent si(indent_size_);
EmitBlock(if_->False(), "false");
@@ -584,9 +579,7 @@
if (!l->Initializer()->IsEmpty()) {
parts.Push("i: %b" + std::to_string(IdOf(l->Initializer())));
}
- if (!l->Body()->IsEmpty()) {
- parts.Push("b: %b" + std::to_string(IdOf(l->Body())));
- }
+ parts.Push("b: %b" + std::to_string(IdOf(l->Body())));
if (!l->Continuing()->IsEmpty()) {
parts.Push("c: %b" + std::to_string(IdOf(l->Continuing())));
@@ -603,7 +596,8 @@
EmitBlock(l->Initializer(), "initializer");
}
- if (!l->Body()->IsEmpty()) {
+ // Loop is assumed to always have a body
+ {
ScopedIndent si(indent_size_);
EmitBlock(l->Body(), "body");
}
diff --git a/src/tint/ir/transform/merge_return_test.cc b/src/tint/ir/transform/merge_return_test.cc
index 0991bb5..2e59642 100644
--- a/src/tint/ir/transform/merge_return_test.cc
+++ b/src/tint/ir/transform/merge_return_test.cc
@@ -100,7 +100,8 @@
auto* swtch = b.Switch(in);
b.With(b.Case(swtch, {Switch::CaseSelector{}}), [&] { b.ExitSwitch(swtch); });
- b.Loop();
+ auto* l = b.Loop();
+ b.With(l->Body(), [&] { b.ExitLoop(l); });
auto* ifelse = b.If(cond);
ifelse->SetResults(b.InstructionResult(ty.i32()));
@@ -118,14 +119,17 @@
exit_switch # switch_1
}
}
- loop [] { # loop_1
+ loop [b: %b3] { # loop_1
+ %b3 = block { # body
+ exit_loop # loop_1
+ }
}
- %3:i32 = if %4 [t: %b3, f: %b4] { # if_1
- %b3 = block { # true
+ %3:i32 = if %4 [t: %b4, f: %b5] { # if_1
+ %b4 = block { # true
%5:i32 = add %2, 1i
exit_if %5 # if_1
}
- %b4 = block { # false
+ %b5 = block { # false
%6:i32 = add %2, 2i
exit_if %6 # if_1
}
@@ -1506,7 +1510,8 @@
b.With(func->Block(), [&] {
auto* sw = b.Switch(cond);
b.With(b.Case(sw, {Switch::CaseSelector{b.Constant(1_i)}}), [&] {
- auto* ifelse = b.If(cond);
+ auto* ifcond = b.Equal(ty.bool_(), cond, 1_i);
+ auto* ifelse = b.If(ifcond);
b.With(ifelse->True(), [&] { b.Return(func, 42_i); });
b.With(ifelse->False(), [&] { b.ExitIf(ifelse); });
@@ -1528,7 +1533,8 @@
%b2 = block {
switch %3 [c: (1i, %b3), c: (default, %b4)] { # switch_1
%b3 = block { # case
- if %3 [t: %b5, f: %b6] { # if_1
+ %4:bool = eq %3, 1i
+ if %4 [t: %b5, f: %b6] { # if_1
%b5 = block { # true
ret 42i
}
@@ -1560,7 +1566,8 @@
%continue_execution:ptr<function, bool, read_write> = var, true
switch %3 [c: (1i, %b3), c: (default, %b4)] { # switch_1
%b3 = block { # case
- if %3 [t: %b5, f: %b6] { # if_1
+ %6:bool = eq %3, 1i
+ if %6 [t: %b5, f: %b6] { # if_1
%b5 = block { # true
store %continue_execution, false
store %return_value, 42i
@@ -1570,8 +1577,8 @@
exit_if # if_1
}
}
- %6:bool = load %continue_execution
- if %6 [t: %b7] { # if_2
+ %7:bool = load %continue_execution
+ if %7 [t: %b7] { # if_2
%b7 = block { # true
store %1, 2i
exit_switch # switch_1
@@ -1583,15 +1590,15 @@
exit_switch # switch_1
}
}
- %7:bool = load %continue_execution
- if %7 [t: %b8] { # if_3
+ %8:bool = load %continue_execution
+ if %8 [t: %b8] { # if_3
%b8 = block { # true
store %return_value, 0i
exit_if # if_3
}
}
- %8:i32 = load %return_value
- ret %8
+ %9:i32 = load %return_value
+ ret %9
}
}
)";
diff --git a/src/tint/ir/validate.cc b/src/tint/ir/validate.cc
index a743786..e43a208 100644
--- a/src/tint/ir/validate.cc
+++ b/src/tint/ir/validate.cc
@@ -35,6 +35,7 @@
#include "src/tint/ir/if.h"
#include "src/tint/ir/load.h"
#include "src/tint/ir/loop.h"
+#include "src/tint/ir/multi_in_block.h"
#include "src/tint/ir/next_iteration.h"
#include "src/tint/ir/return.h"
#include "src/tint/ir/store.h"
@@ -266,9 +267,9 @@
[&](Call* c) { CheckCall(c); }, //
[&](If* if_) { CheckIf(if_); }, //
[&](Load*) {}, //
- [&](Loop*) {}, //
+ [&](Loop* l) { CheckLoop(l); }, //
[&](Store*) {}, //
- [&](Switch*) {}, //
+ [&](Switch* s) { CheckSwitch(s); }, //
[&](Swizzle*) {}, //
[&](Terminator* b) { CheckTerminator(b); }, //
[&](Unary* u) { CheckUnary(u); }, //
@@ -405,6 +406,28 @@
if (if_->Condition() && !if_->Condition()->Type()->Is<type::Bool>()) {
AddError(if_, If::kConditionOperandOffset, "if: condition must be a `bool` type");
}
+
+ CheckBlock(if_->True());
+ if (!if_->False()->IsEmpty()) {
+ CheckBlock(if_->False());
+ }
+ }
+
+ void CheckLoop(Loop* l) {
+ if (!l->Initializer()->IsEmpty()) {
+ CheckBlock(l->Initializer());
+ }
+ CheckBlock(l->Body());
+
+ if (!l->Continuing()->IsEmpty()) {
+ CheckBlock(l->Continuing());
+ }
+ }
+
+ void CheckSwitch(Switch* s) {
+ for (auto& cse : s->Cases()) {
+ CheckBlock(cse.block);
+ }
}
void CheckVar(Var* var) {
diff --git a/src/tint/ir/validate_test.cc b/src/tint/ir/validate_test.cc
index 723c3fc..db8079f 100644
--- a/src/tint/ir/validate_test.cc
+++ b/src/tint/ir/validate_test.cc
@@ -515,6 +515,50 @@
)");
}
+TEST_F(IR_ValidateTest, If_EmptyFalse) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto* if_ = b.If(true);
+ if_->True()->Append(b.Return(f));
+
+ f->Block()->Append(if_);
+ f->Block()->Append(b.Return(f));
+
+ auto res = ir::Validate(mod);
+ EXPECT_TRUE(res) << res.Failure().str();
+}
+
+TEST_F(IR_ValidateTest, If_EmptyTrue) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto* if_ = b.If(true);
+ if_->False()->Append(b.Return(f));
+
+ f->Block()->Append(if_);
+ f->Block()->Append(b.Return(f));
+
+ auto res = ir::Validate(mod);
+ ASSERT_FALSE(res);
+ EXPECT_EQ(res.Failure().str(), R"(:4:7 error: block: does not end in a terminator instruction
+ %b2 = block { # true
+ ^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ if true [t: %b2, f: %b3] { # if_1
+ %b2 = block { # true
+ }
+ %b3 = block { # false
+ ret
+ }
+ }
+ ret
+ }
+}
+)");
+}
+
TEST_F(IR_ValidateTest, If_ConditionIsBool) {
auto* f = b.Function("my_func", ty.void_());
@@ -589,6 +633,46 @@
)");
}
+TEST_F(IR_ValidateTest, Loop_OnlyBody) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto* l = b.Loop();
+ l->Body()->Append(b.ExitLoop(l));
+
+ auto sb = b.With(f->Block());
+ sb.Append(l);
+ sb.Return(f);
+
+ auto res = ir::Validate(mod);
+ EXPECT_TRUE(res) << res.Failure().str();
+}
+
+TEST_F(IR_ValidateTest, Loop_EmptyBody) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto sb = b.With(f->Block());
+ sb.Append(b.Loop());
+ sb.Return(f);
+
+ auto res = ir::Validate(mod);
+ ASSERT_FALSE(res);
+ EXPECT_EQ(res.Failure().str(), R"(:4:7 error: block: does not end in a terminator instruction
+ %b2 = block { # body
+ ^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ loop [b: %b2] { # loop_1
+ %b2 = block { # body
+ }
+ }
+ ret
+ }
+}
+)");
+}
+
TEST_F(IR_ValidateTest, Var_RootBlock_NullResult) {
auto* v = mod.instructions.Create<ir::Var>(nullptr);
b.RootBlock()->Append(v);
