[ir] Validate block parameters
Check that they are alive and have the correct parent block.
Add source mapping for block parameters to the disassembler, and
remove `EmitValueList()` which is no longer used.
Change-Id: I392506d187ab558cc27df5676ea3f46bc52268c8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/185524
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@google.com>
Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/tint/lang/core/ir/disassembler.cc b/src/tint/lang/core/ir/disassembler.cc
index d2cd83c..281cb61 100644
--- a/src/tint/lang/core/ir/disassembler.cc
+++ b/src/tint/lang/core/ir/disassembler.cc
@@ -188,7 +188,14 @@
if (auto* merge = blk->As<MultiInBlock>()) {
if (!merge->Params().IsEmpty()) {
out_ << " (";
- EmitValueList(merge->Params().Slice());
+ for (auto* p : merge->Params()) {
+ if (p != merge->Params().Front()) {
+ out_ << ", ";
+ }
+ SourceMarker psm(this);
+ EmitValue(p);
+ psm.Store(p);
+ }
out_ << ")";
}
}
@@ -797,15 +804,6 @@
);
}
-void Disassembler::EmitValueList(tint::Slice<const Value* const> values) {
- for (size_t i = 0, n = values.Length(); i < n; i++) {
- if (i > 0) {
- out_ << ", ";
- }
- EmitValue(values[i]);
- }
-}
-
void Disassembler::EmitBinary(const Binary* b) {
SourceMarker sm(this);
EmitValueWithType(b);
diff --git a/src/tint/lang/core/ir/disassembler.h b/src/tint/lang/core/ir/disassembler.h
index a2372d4..6b6f7c6 100644
--- a/src/tint/lang/core/ir/disassembler.h
+++ b/src/tint/lang/core/ir/disassembler.h
@@ -32,6 +32,7 @@
#include "src/tint/lang/core/ir/binary.h"
#include "src/tint/lang/core/ir/block.h"
+#include "src/tint/lang/core/ir/block_param.h"
#include "src/tint/lang/core/ir/call.h"
#include "src/tint/lang/core/ir/if.h"
#include "src/tint/lang/core/ir/loop.h"
@@ -104,6 +105,12 @@
/// @returns the source for the block
Source BlockSource(const Block* blk) { return block_to_src_.GetOr(blk, Source{}); }
+ /// @param param the block parameter to retrieve
+ /// @returns the source for the parameter
+ Source BlockParamSource(const BlockParam* param) {
+ return block_param_to_src_.GetOr(param, Source{});
+ }
+
/// @param func the function to retrieve
/// @returns the source for the function
Source FunctionSource(const Function* func) { return function_to_src_.GetOr(func, Source{}); }
@@ -124,6 +131,11 @@
/// @param src the source location
void SetSource(const Block* blk, Source src) { block_to_src_.Add(blk, src); }
+ /// Stores the given @p src location for @p param block parameter
+ /// @param param the block parameter to store
+ /// @param src the source location
+ void SetSource(const BlockParam* param, Source src) { block_param_to_src_.Add(param, src); }
+
/// Stores the given @p src location for @p func function
/// @param func the function to store
/// @param src the source location
@@ -159,6 +171,8 @@
void Store(const Block* blk) { dis_->SetSource(blk, MakeSource()); }
+ void Store(const BlockParam* param) { dis_->SetSource(param, MakeSource()); }
+
void Store(const Function* func) { dis_->SetSource(func, MakeSource()); }
void Store(const FunctionParam* param) { dis_->SetSource(param, MakeSource()); }
@@ -194,7 +208,6 @@
void EmitValueWithType(const Instruction* val);
void EmitValueWithType(const Value* val);
void EmitValue(const Value* val);
- void EmitValueList(tint::Slice<const ir::Value* const> values);
void EmitBinary(const Binary* b);
void EmitUnary(const Unary* b);
void EmitTerminator(const Terminator* b);
@@ -219,6 +232,7 @@
uint32_t current_output_start_pos_ = 0;
Hashmap<const Block*, Source, 8> block_to_src_;
+ Hashmap<const BlockParam*, Source, 8> block_param_to_src_;
Hashmap<const Instruction*, Source, 8> instruction_to_src_;
Hashmap<IndexedValue, Source, 8> operand_to_src_;
Hashmap<IndexedValue, Source, 8> result_to_src_;
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index 3634c7b..24d594d 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -163,6 +163,11 @@
/// @returns the diagnostic
diag::Diagnostic& AddError(const Block* blk);
+ /// Adds an error for the @p param and highlights the parameter in the disassembly
+ /// @param param the parameter
+ /// @returns the diagnostic
+ diag::Diagnostic& AddError(const BlockParam* param);
+
/// Adds an error for the @p func and highlights the function in the disassembly
/// @param func the function
/// @returns the diagnostic
@@ -427,6 +432,12 @@
return AddError(src);
}
+diag::Diagnostic& Validator::AddError(const BlockParam* param) {
+ DisassembleIfNeeded();
+ auto src = dis_.BlockParamSource(param);
+ return AddError(src);
+}
+
diag::Diagnostic& Validator::AddError(const Function* func) {
DisassembleIfNeeded();
auto src = dis_.FunctionSource(func);
@@ -547,6 +558,23 @@
void Validator::CheckBlock(const Block* blk) {
TINT_SCOPED_ASSIGNMENT(current_block_, blk);
+ if (auto* mb = blk->As<MultiInBlock>()) {
+ for (auto* param : mb->Params()) {
+ if (!param->Alive()) {
+ AddError(param) << "destroyed parameter found in block parameter list";
+ return;
+ }
+ if (!param->Block()) {
+ AddError(param) << "block parameter has nullptr parent block";
+ return;
+ } else if (param->Block() != mb) {
+ AddError(param) << "block parameter has incorrect parent block";
+ AddNote(param->Block()) << "parent block declared here";
+ return;
+ }
+ }
+ }
+
if (!blk->Terminator()) {
AddError(blk) << "block: does not end in a terminator instruction";
}
diff --git a/src/tint/lang/core/ir/validator_test.cc b/src/tint/lang/core/ir/validator_test.cc
index 681a6b2..8742f8a 100644
--- a/src/tint/lang/core/ir/validator_test.cc
+++ b/src/tint/lang/core/ir/validator_test.cc
@@ -478,6 +478,115 @@
)");
}
+TEST_F(IR_ValidatorTest, Block_DeadParameter) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto* p = b.BlockParam("my_param", ty.f32());
+ b.Append(f->Block(), [&] {
+ auto* l = b.Loop();
+ l->Body()->SetParams({p});
+ b.Append(l->Body(), [&] { b.ExitLoop(l); });
+ b.Return(f);
+ });
+
+ p->Destroy();
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.Str(),
+ R"(:4:20 error: destroyed parameter found in block parameter list
+ %b2 = block (%my_param:f32) { # body
+ ^^^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ loop [b: %b2] { # loop_1
+ %b2 = block (%my_param:f32) { # body
+ exit_loop # loop_1
+ }
+ }
+ ret
+ }
+}
+)");
+}
+
+TEST_F(IR_ValidatorTest, Block_ParameterWithNullBlock) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto* p = b.BlockParam("my_param", ty.f32());
+ b.Append(f->Block(), [&] {
+ auto* l = b.Loop();
+ l->Body()->SetParams({p});
+ b.Append(l->Body(), [&] { b.ExitLoop(l); });
+ b.Return(f);
+ });
+
+ p->SetBlock(nullptr);
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.Str(),
+ R"(:4:20 error: block parameter has nullptr parent block
+ %b2 = block (%my_param:f32) { # body
+ ^^^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ loop [b: %b2] { # loop_1
+ %b2 = block (%my_param:f32) { # body
+ exit_loop # loop_1
+ }
+ }
+ ret
+ }
+}
+)");
+}
+
+TEST_F(IR_ValidatorTest, Block_ParameterUsedInMultipleBlocks) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ auto* p = b.BlockParam("my_param", ty.f32());
+ b.Append(f->Block(), [&] {
+ auto* l = b.Loop();
+ l->Body()->SetParams({p});
+ b.Append(l->Body(), [&] { b.Continue(l, p); });
+ l->Continuing()->SetParams({p});
+ b.Append(l->Continuing(), [&] { b.NextIteration(l, p); });
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.Str(),
+ R"(:4:20 error: block parameter has incorrect parent block
+ %b2 = block (%my_param:f32) { # body
+ ^^^^^^^^^^^^^
+
+:7:7 note: parent block declared here
+ %b3 = block (%my_param:f32) { # continuing
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ loop [b: %b2, c: %b3] { # loop_1
+ %b2 = block (%my_param:f32) { # body
+ continue %b3 %my_param:f32
+ }
+ %b3 = block (%my_param:f32) { # continuing
+ next_iteration %b2 %my_param:f32
+ }
+ }
+ ret
+ }
+}
+)");
+}
+
TEST_F(IR_ValidatorTest, Access_NegativeIndex) {
auto* f = b.Function("my_func", ty.void_());
auto* obj = b.FunctionParam(ty.vec3<f32>());
