[ir] Validate load instructions
Remove assertions from Load constructors to avoid redundancy and to
match store.
Bug: tint:1952
Change-Id: I2a557d13237361fe1ea55ab9620a4764a21768f1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/169120
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/tint/lang/core/ir/load.cc b/src/tint/lang/core/ir/load.cc
index 6563ea1..1cdca31 100644
--- a/src/tint/lang/core/ir/load.cc
+++ b/src/tint/lang/core/ir/load.cc
@@ -43,9 +43,6 @@
Load::Load(InstructionResult* result, Value* from) {
flags_.Add(Flag::kSequenced);
- TINT_ASSERT(from->Type()->Is<core::type::Pointer>());
- TINT_ASSERT(from && from->Type()->UnwrapPtr() == result->Type());
-
AddOperand(Load::kFromOperandOffset, from);
AddResult(result);
}
diff --git a/src/tint/lang/core/ir/load.h b/src/tint/lang/core/ir/load.h
index 987b4b1..6b65a1d 100644
--- a/src/tint/lang/core/ir/load.h
+++ b/src/tint/lang/core/ir/load.h
@@ -44,7 +44,7 @@
/// Constructor (no results, no operands)
Load();
- /// Constructor (infers type)
+ /// Constructor
/// @param result the result value
/// @param from the value being loaded from
Load(InstructionResult* result, Value* from);
diff --git a/src/tint/lang/core/ir/load_test.cc b/src/tint/lang/core/ir/load_test.cc
index 1550b14..bccaa13 100644
--- a/src/tint/lang/core/ir/load_test.cc
+++ b/src/tint/lang/core/ir/load_test.cc
@@ -71,16 +71,6 @@
EXPECT_EQ(inst->Result(0)->Instruction(), inst);
}
-TEST_F(IR_LoadTest, Fail_NonPtr_Builder) {
- EXPECT_FATAL_FAILURE(
- {
- Module mod;
- Builder b{mod};
- b.Load(b.Constant(1_i));
- },
- "");
-}
-
TEST_F(IR_LoadTest, Clone) {
auto* var = b.Var(ty.ptr<function, i32>());
auto* inst = b.Load(var);
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index b6e96e5..8916672 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -263,6 +263,10 @@
/// @param l the exit loop to validate
void CheckExitLoop(const ExitLoop* l);
+ /// Validates the given load
+ /// @param l the load to validate
+ void CheckLoad(const Load* l);
+
/// Validates the given store
/// @param s the store to validate
void CheckStore(const Store* s);
@@ -521,7 +525,7 @@
[&](const Call* c) { CheckCall(c); }, //
[&](const If* if_) { CheckIf(if_); }, //
[&](const Let* let) { CheckLet(let); }, //
- [&](const Load*) {}, //
+ [&](const Load* load) { CheckLoad(load); }, //
[&](const LoadVectorElement* l) { CheckLoadVectorElement(l); }, //
[&](const Loop* l) { CheckLoop(l); }, //
[&](const Store* s) { CheckStore(s); }, //
@@ -916,6 +920,21 @@
}
}
+void Validator::CheckLoad(const Load* l) {
+ CheckOperandNotNull(l, l->From(), Load::kFromOperandOffset);
+
+ if (auto* from = l->From()) {
+ auto* mv = from->Type()->As<core::type::MemoryView>();
+ if (!mv) {
+ AddError(l, Load::kFromOperandOffset, "load source operand is not a memory view");
+ return;
+ }
+ if (l->Result(0)->Type() != mv->StoreType()) {
+ AddError(l, Load::kFromOperandOffset, "result type does not match source store type");
+ }
+ }
+}
+
void Validator::CheckStore(const Store* s) {
CheckOperandsNotNull(s, Store::kToOperandOffset, Store::kFromOperandOffset);
diff --git a/src/tint/lang/core/ir/validator_test.cc b/src/tint/lang/core/ir/validator_test.cc
index 9989e2a..7da4afa 100644
--- a/src/tint/lang/core/ir/validator_test.cc
+++ b/src/tint/lang/core/ir/validator_test.cc
@@ -2986,6 +2986,96 @@
)");
}
+TEST_F(IR_ValidatorTest, Load_NullFrom) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ b.Append(f->Block(), [&] {
+ b.Append(mod.instructions.Create<ir::Load>(b.InstructionResult(ty.i32()), nullptr));
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.str(), R"(:3:19 error: load: operand is undefined
+ %2:i32 = load undef
+ ^^^^^
+
+:2:3 note: In block
+ %b1 = block {
+ ^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ %2:i32 = load undef
+ ret
+ }
+}
+)");
+}
+
+TEST_F(IR_ValidatorTest, Load_SourceNotMemoryView) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ b.Append(f->Block(), [&] {
+ auto* let = b.Let("l", 1_i);
+ b.Append(mod.instructions.Create<ir::Load>(b.InstructionResult(ty.f32()), let->Result(0)));
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.str(),
+ R"(:4:19 error: load source operand is not a memory view
+ %3:f32 = load %l
+ ^^
+
+:2:3 note: In block
+ %b1 = block {
+ ^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ %l:i32 = let 1i
+ %3:f32 = load %l
+ ret
+ }
+}
+)");
+}
+
+TEST_F(IR_ValidatorTest, Load_TypeMismatch) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ b.Append(f->Block(), [&] {
+ auto* var = b.Var(ty.ptr<function, i32>());
+ b.Append(mod.instructions.Create<ir::Load>(b.InstructionResult(ty.f32()), var->Result(0)));
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.str(),
+ R"(:4:19 error: result type does not match source store type
+ %3:f32 = load %2
+ ^^
+
+:2:3 note: In block
+ %b1 = block {
+ ^^^^^^^^^^^
+
+note: # Disassembly
+%my_func = func():void -> %b1 {
+ %b1 = block {
+ %2:ptr<function, i32, read_write> = var
+ %3:f32 = load %2
+ ret
+ }
+}
+)");
+}
+
TEST_F(IR_ValidatorTest, Store_NullTo) {
auto* f = b.Function("my_func", ty.void_());