[tint][ir][val] Add checking types for `let`s
Also adds capability to loosen restrictions for MSL
Fixes: 393153114, 394052229
Change-Id: I2299fbb7ed25d413f8dea6a4db75486d55475c37
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/223856
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
diff --git a/src/tint/lang/core/ir/transform/remove_terminator_args.h b/src/tint/lang/core/ir/transform/remove_terminator_args.h
index 2aa83a8..96a1690 100644
--- a/src/tint/lang/core/ir/transform/remove_terminator_args.h
+++ b/src/tint/lang/core/ir/transform/remove_terminator_args.h
@@ -47,6 +47,7 @@
core::ir::Capability::kAllowHandleVarsWithoutBindings,
core::ir::Capability::kAllowClipDistancesOnF32,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
};
/// RemoveTerminatorArgs is a transform that removes all arguments from terminator instructions and
diff --git a/src/tint/lang/core/ir/transform/rename_conflicts.h b/src/tint/lang/core/ir/transform/rename_conflicts.h
index 579834f..1b608a0 100644
--- a/src/tint/lang/core/ir/transform/rename_conflicts.h
+++ b/src/tint/lang/core/ir/transform/rename_conflicts.h
@@ -47,6 +47,7 @@
core::ir::Capability::kAllowHandleVarsWithoutBindings,
core::ir::Capability::kAllowClipDistancesOnF32,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
};
/// RenameConflicts is a transform that renames declarations which prevent identifiers from
diff --git a/src/tint/lang/core/ir/transform/value_to_let.h b/src/tint/lang/core/ir/transform/value_to_let.h
index ac088bb..5127597 100644
--- a/src/tint/lang/core/ir/transform/value_to_let.h
+++ b/src/tint/lang/core/ir/transform/value_to_let.h
@@ -48,6 +48,7 @@
core::ir::Capability::kAllowHandleVarsWithoutBindings,
core::ir::Capability::kAllowClipDistancesOnF32,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
};
/// Configuration for ValueToLet transform.
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index e2e490d..062f973 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -2644,17 +2644,31 @@
return;
}
- if (l->Result(0)->Type()->Is<core::type::Void>()) {
- AddError(l) << "result type cannot be void";
- return;
+ auto* value_ty = l->Value()->Type();
+ if (capabilities_.Contains(Capability::kAllowAnyLetType)) {
+ if (value_ty->Is<core::type::Void>()) {
+ AddError(l) << "value type cannot be void";
+ }
+ } else {
+ if (!value_ty->IsConstructible() && !value_ty->Is<core::type::Pointer>()) {
+ AddError(l) << "value type, " << NameOf(value_ty)
+ << ", must be concrete constructible type or a pointer type";
+ }
}
- if (l->Value()->Type()->Is<core::type::Void>()) {
- AddError(l) << "value type cannot be void";
- return;
+ auto* result_ty = l->Result(0)->Type();
+ if (capabilities_.Contains(Capability::kAllowAnyLetType)) {
+ if (result_ty->Is<core::type::Void>()) {
+ AddError(l) << "result type cannot be void";
+ }
+ } else {
+ if (!result_ty->IsConstructible() && !result_ty->Is<core::type::Pointer>()) {
+ AddError(l) << "result type, " << NameOf(result_ty)
+ << ", must be concrete constructible type or a pointer type";
+ }
}
- if (l->Result(0)->Type() != l->Value()->Type()) {
+ if (value_ty != result_ty) {
AddError(l) << "result type " << NameOf(l->Result(0)->Type())
<< " does not match value type " << NameOf(l->Value()->Type());
}
diff --git a/src/tint/lang/core/ir/validator.h b/src/tint/lang/core/ir/validator.h
index 4424f74..fa2a373 100644
--- a/src/tint/lang/core/ir/validator.h
+++ b/src/tint/lang/core/ir/validator.h
@@ -64,6 +64,8 @@
kAllowPrivateVarsInFunctions,
/// Allows phony assignment instructions to be used.
kAllowPhonyInstructions,
+ /// Allows lets to have any type, used by MSL backend for module scoped vars
+ kAllowAnyLetType,
};
/// Capabilities is a set of Capability
diff --git a/src/tint/lang/core/ir/validator_value_test.cc b/src/tint/lang/core/ir/validator_value_test.cc
index ec8948a..7ce9d42 100644
--- a/src/tint/lang/core/ir/validator_value_test.cc
+++ b/src/tint/lang/core/ir/validator_value_test.cc
@@ -583,7 +583,24 @@
)")) << res.Failure().reason.Str();
}
-TEST_F(IR_ValidatorTest, Let_VoidResult) {
+TEST_F(IR_ValidatorTest, Let_VoidResultWithCapability) {
+ auto* f = b.Function("my_func", ty.void_());
+ b.Append(f->Block(), [&] {
+ auto* l = mod.CreateInstruction<ir::Let>(b.InstructionResult(ty.void_()), b.Constant(1_i));
+ b.Append(l);
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod, Capabilities{ir::Capability::kAllowAnyLetType});
+ ASSERT_NE(res, Success);
+ EXPECT_THAT(res.Failure().reason.Str(), testing::HasSubstr(
+ R"(:3:15 error: let: result type cannot be void
+ %2:void = let 1i
+ ^^^
+)")) << res.Failure().reason.Str();
+}
+
+TEST_F(IR_ValidatorTest, Let_VoidResultWithoutCapability) {
auto* f = b.Function("my_func", ty.void_());
b.Append(f->Block(), [&] {
auto* l = mod.CreateInstruction<ir::Let>(b.InstructionResult(ty.void_()), b.Constant(1_i));
@@ -593,14 +610,36 @@
auto res = ir::Validate(mod);
ASSERT_NE(res, Success);
- EXPECT_THAT(res.Failure().reason.Str(),
- testing::HasSubstr(R"(:3:15 error: let: result type cannot be void
+ EXPECT_THAT(
+ res.Failure().reason.Str(),
+ testing::HasSubstr(
+ R"(:3:15 error: let: result type, 'void', must be concrete constructible type or a pointer type
%2:void = let 1i
^^^
)")) << res.Failure().reason.Str();
}
-TEST_F(IR_ValidatorTest, Let_VoidValue) {
+TEST_F(IR_ValidatorTest, Let_VoidValueWithCapability) {
+ auto* v = b.Function("void_func", ty.void_());
+ b.Append(v->Block(), [&] { b.Return(v); });
+
+ auto* f = b.Function("my_func", ty.void_());
+ b.Append(f->Block(), [&] {
+ auto* l = mod.CreateInstruction<ir::Let>(b.InstructionResult(ty.i32()), b.Value(b.Call(v)));
+ b.Append(l);
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod, Capabilities{ir::Capability::kAllowAnyLetType});
+ ASSERT_NE(res, Success);
+ EXPECT_THAT(res.Failure().reason.Str(),
+ testing::HasSubstr(R"(:9:14 error: let: value type cannot be void
+ %4:i32 = let %3
+ ^^^
+)")) << res.Failure().reason.Str();
+}
+
+TEST_F(IR_ValidatorTest, Let_VoidValueWithoutCapability) {
auto* v = b.Function("void_func", ty.void_());
b.Append(v->Block(), [&] { b.Return(v); });
@@ -613,13 +652,73 @@
auto res = ir::Validate(mod);
ASSERT_NE(res, Success);
- EXPECT_THAT(res.Failure().reason.Str(),
- testing::HasSubstr(R"(:9:14 error: let: value type cannot be void
+ EXPECT_THAT(
+ res.Failure().reason.Str(),
+ testing::HasSubstr(
+ R"(:9:14 error: let: value type, 'void', must be concrete constructible type or a pointer type
%4:i32 = let %3
^^^
)")) << res.Failure().reason.Str();
}
+TEST_F(IR_ValidatorTest, Let_NotConstructibleResult) {
+ auto* f = b.Function("my_func", ty.void_());
+ auto* p = b.FunctionParam("p", ty.sampler());
+ f->AppendParam(p);
+ b.Append(f->Block(), [&] {
+ auto* l =
+ mod.CreateInstruction<ir::Let>(b.InstructionResult(ty.sampler()), b.Constant(1_i));
+ b.Append(l);
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_THAT(
+ res.Failure().reason.Str(),
+ testing::HasSubstr(
+ R"(:3:18 error: let: result type, 'sampler', must be concrete constructible type or a pointer type
+ %3:sampler = let 1i
+ ^^^
+)")) << res.Failure().reason.Str();
+}
+
+TEST_F(IR_ValidatorTest, Let_NotConstructibleValue) {
+ auto* f = b.Function("my_func", ty.void_());
+ auto* p = b.FunctionParam("p", ty.sampler());
+ f->AppendParam(p);
+ b.Append(f->Block(), [&] {
+ auto* l = mod.CreateInstruction<ir::Let>(b.InstructionResult(ty.i32()), p);
+ b.Append(l);
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_THAT(
+ res.Failure().reason.Str(),
+ testing::HasSubstr(
+ R"(:3:14 error: let: value type, 'sampler', must be concrete constructible type or a pointer type
+ %3:i32 = let %p
+ ^^^
+)")) << res.Failure().reason.Str();
+}
+
+TEST_F(IR_ValidatorTest, Let_CapabilityBypass) {
+ auto* f = b.Function("my_func", ty.void_());
+ auto* p = b.FunctionParam("p", ty.sampler());
+ f->AppendParam(p);
+
+ b.Append(f->Block(), [&] {
+ auto* l = mod.CreateInstruction<ir::Let>(b.InstructionResult(ty.sampler()), p);
+ b.Append(l);
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod, Capabilities{ir::Capability::kAllowAnyLetType});
+ ASSERT_EQ(res, Success) << res.Failure().reason.Str();
+}
+
TEST_F(IR_ValidatorTest, Phony_NullValue) {
auto* v = mod.CreateInstruction<ir::Phony>(nullptr);
auto* f = b.Function("my_func", ty.void_());
diff --git a/src/tint/lang/msl/writer/printer/printer.cc b/src/tint/lang/msl/writer/printer/printer.cc
index 1c57ba0..496d789 100644
--- a/src/tint/lang/msl/writer/printer/printer.cc
+++ b/src/tint/lang/msl/writer/printer/printer.cc
@@ -131,6 +131,7 @@
core::ir::Capability::kAllow64BitIntegers,
core::ir::Capability::kAllowPointersAndHandlesInStructures,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
});
if (valid != Success) {
return std::move(valid.Failure());
diff --git a/src/tint/lang/msl/writer/raise/binary_polyfill.cc b/src/tint/lang/msl/writer/raise/binary_polyfill.cc
index 8932b88..6ed02d6 100644
--- a/src/tint/lang/msl/writer/raise/binary_polyfill.cc
+++ b/src/tint/lang/msl/writer/raise/binary_polyfill.cc
@@ -162,6 +162,7 @@
core::ir::Capabilities{
core::ir::Capability::kAllowPointersAndHandlesInStructures,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
});
if (result != Success) {
return result.Failure();
diff --git a/src/tint/lang/msl/writer/raise/builtin_polyfill.cc b/src/tint/lang/msl/writer/raise/builtin_polyfill.cc
index e90a137..2581f69 100644
--- a/src/tint/lang/msl/writer/raise/builtin_polyfill.cc
+++ b/src/tint/lang/msl/writer/raise/builtin_polyfill.cc
@@ -1039,6 +1039,7 @@
core::ir::Capabilities{
core::ir::Capability::kAllowPointersAndHandlesInStructures,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
});
if (result != Success) {
return result.Failure();
diff --git a/src/tint/lang/msl/writer/raise/module_scope_vars_test.cc b/src/tint/lang/msl/writer/raise/module_scope_vars_test.cc
index f8c1dc5..4b1c991 100644
--- a/src/tint/lang/msl/writer/raise/module_scope_vars_test.cc
+++ b/src/tint/lang/msl/writer/raise/module_scope_vars_test.cc
@@ -43,7 +43,8 @@
public:
void SetUp() override {
capabilities.Add(core::ir::Capability::kAllowPointersAndHandlesInStructures,
- core::ir::Capability::kAllowPrivateVarsInFunctions);
+ core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType);
}
};
diff --git a/src/tint/lang/msl/writer/raise/unary_polyfill.cc b/src/tint/lang/msl/writer/raise/unary_polyfill.cc
index 812ccbc..e116942 100644
--- a/src/tint/lang/msl/writer/raise/unary_polyfill.cc
+++ b/src/tint/lang/msl/writer/raise/unary_polyfill.cc
@@ -93,6 +93,7 @@
core::ir::Capabilities{
core::ir::Capability::kAllowPointersAndHandlesInStructures,
core::ir::Capability::kAllowPrivateVarsInFunctions,
+ core::ir::Capability::kAllowAnyLetType,
});
if (result != Success) {
return result.Failure();
