[tint][ir][val] Clean up HasPositionAndLocation checks
This moves these to only be tested for entry point related things,
i.e. input params, function returns, and MSVs in the in and out space,
so it is no longer failing valid shaders.
There is further work needed to clean up the general handling of vars
+ attributes, https://issues.chromium.org/issues/378666657
Fixes: 378168373
Change-Id: Ia0c720230d176623e5fb8d997e0fe4a46cb1266a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/214735
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index 6ee5008..3abb1dc 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -1859,12 +1859,15 @@
CheckInvariantFunc<FunctionParam>(
"invariant can only decorate a param member iff it is also "
"decorated with position"));
- CheckFunctionParamAttributes(
- param,
- CheckDoesNotHaveBothLocationAndBuiltinFunc<FunctionParam>(
- "a builtin and location cannot be both declared for a param"),
- CheckDoesNotHaveBothLocationAndBuiltinFunc<FunctionParam>(
- "a builtin and location cannot be both declared for a struct member"));
+
+ if (func->Stage() != Function::PipelineStage::kUndefined) {
+ CheckFunctionParamAttributes(
+ param,
+ CheckDoesNotHaveBothLocationAndBuiltinFunc<FunctionParam>(
+ "a builtin and location cannot be both declared for a param"),
+ CheckDoesNotHaveBothLocationAndBuiltinFunc<FunctionParam>(
+ "a builtin and location cannot be both declared for a struct member"));
+ }
if (func->Stage() == Function::PipelineStage::kFragment) {
CheckFunctionParamAttributesAndType(
@@ -1898,12 +1901,14 @@
CheckInvariantFunc<Function>(
"invariant can only decorate output members iff they are also position builtins"));
- CheckFunctionReturnAttributes(
- func,
- CheckDoesNotHaveBothLocationAndBuiltinFunc<Function>(
- "a builtin and location cannot be both declared for a function return"),
- CheckDoesNotHaveBothLocationAndBuiltinFunc<Function>(
- "a builtin and location cannot be both declared for a struct member"));
+ if (func->Stage() != Function::PipelineStage::kUndefined) {
+ CheckFunctionReturnAttributes(
+ func,
+ CheckDoesNotHaveBothLocationAndBuiltinFunc<Function>(
+ "a builtin and location cannot be both declared for a function return"),
+ CheckDoesNotHaveBothLocationAndBuiltinFunc<Function>(
+ "a builtin and location cannot be both declared for a struct member"));
+ }
// void needs to be filtered out, since it isn't constructible, but used in the IR when no
// return is specified.
@@ -2295,18 +2300,23 @@
return;
}
- if (HasLocationAndBuiltin(var->Attributes())) {
- AddError(var) << "a builtin and location cannot be both declared for a var";
- return;
- }
-
- if (auto* s = var->Result(0)->Type()->UnwrapPtrOrRef()->As<core::type::Struct>()) {
- for (auto* mem : s->Members()) {
- if (HasLocationAndBuiltin(mem->Attributes())) {
+ if (var->Block() == mod_.root_block) {
+ if (mv->AddressSpace() == AddressSpace::kIn || mv->AddressSpace() == AddressSpace::kOut) {
+ if (HasLocationAndBuiltin(var->Attributes())) {
AddError(var)
- << "a builtin and location cannot be both declared for a struct member";
+ << "a builtin and location cannot be both declared for a module scope var";
return;
}
+
+ if (auto* s = var->Result(0)->Type()->UnwrapPtrOrRef()->As<core::type::Struct>()) {
+ for (auto* mem : s->Members()) {
+ if (HasLocationAndBuiltin(mem->Attributes())) {
+ AddError(var) << "a builtin and location cannot be both declared for a "
+ "module scope var struct member";
+ return;
+ }
+ }
+ }
}
}
}
diff --git a/src/tint/lang/core/ir/validator_test.cc b/src/tint/lang/core/ir/validator_test.cc
index 5f030e1..81bc844 100644
--- a/src/tint/lang/core/ir/validator_test.cc
+++ b/src/tint/lang/core/ir/validator_test.cc
@@ -5151,40 +5151,36 @@
)");
}
-TEST_F(IR_ValidatorTest, Var_Basic_BothLocationAndBuiltin) {
- auto* f = b.Function("my_func", ty.void_());
-
- b.Append(f->Block(), [&] {
- auto* v = b.Var<function, f32>();
- IOAttributes attr;
- attr.builtin = BuiltinValue::kPosition;
- attr.location = 0;
- v->SetAttributes(attr);
- b.Return(f);
- });
+TEST_F(IR_ValidatorTest, Var_IOBothLocationAndBuiltin) {
+ auto* v = b.Var<AddressSpace::kIn, vec4<f32>>();
+ IOAttributes attr;
+ attr.builtin = BuiltinValue::kPosition;
+ attr.location = 0;
+ v->SetAttributes(attr);
+ v->SetBindingPoint(0, 0);
+ mod.root_block->Append(v);
auto res = ir::Validate(mod);
ASSERT_NE(res, Success);
- EXPECT_EQ(res.Failure().reason.Str(),
- R"(:3:41 error: var: a builtin and location cannot be both declared for a var
- %2:ptr<function, f32, read_write> = var @location(0) @builtin(position)
- ^^^
+ EXPECT_EQ(
+ res.Failure().reason.Str(),
+ R"(:2:35 error: var: a builtin and location cannot be both declared for a module scope var
+ %1:ptr<__in, vec4<f32>, read> = var @binding_point(0, 0) @location(0) @builtin(position)
+ ^^^
-:2:3 note: in block
- $B1: {
- ^^^
+:1:1 note: in block
+$B1: { # root
+^^^
note: # Disassembly
-%my_func = func():void {
- $B1: {
- %2:ptr<function, f32, read_write> = var @location(0) @builtin(position)
- ret
- }
+$B1: { # root
+ %1:ptr<__in, vec4<f32>, read> = var @binding_point(0, 0) @location(0) @builtin(position)
}
+
)");
}
-TEST_F(IR_ValidatorTest, Var_Struct_BothLocationAndBuiltin) {
+TEST_F(IR_ValidatorTest, Var_Struct_IOBothLocationAndBuiltin) {
IOAttributes attr;
attr.builtin = BuiltinValue::kPosition;
attr.location = 0;
@@ -5193,21 +5189,17 @@
ty.Struct(mod.symbols.New("MyStruct"), {
{mod.symbols.New("a"), ty.f32(), attr},
});
- auto* v = b.Var(ty.ptr(storage, str_ty, read_write));
+ auto* v = b.Var(ty.ptr(AddressSpace::kOut, str_ty, read_write));
v->SetBindingPoint(0, 0);
mod.root_block->Append(v);
- auto* f = b.Function("my_func", ty.void_());
-
- b.Append(f->Block(), [&] { b.Return(f); });
-
auto res = ir::Validate(mod);
ASSERT_NE(res, Success);
EXPECT_EQ(
res.Failure().reason.Str(),
- R"(:6:43 error: var: a builtin and location cannot be both declared for a struct member
- %1:ptr<storage, MyStruct, read_write> = var @binding_point(0, 0)
- ^^^
+ R"(:6:41 error: var: a builtin and location cannot be both declared for a module scope var struct member
+ %1:ptr<__out, MyStruct, read_write> = var @binding_point(0, 0)
+ ^^^
:5:1 note: in block
$B1: { # root
@@ -5219,14 +5211,9 @@
}
$B1: { # root
- %1:ptr<storage, MyStruct, read_write> = var @binding_point(0, 0)
+ %1:ptr<__out, MyStruct, read_write> = var @binding_point(0, 0)
}
-%my_func = func():void {
- $B2: {
- ret
- }
-}
)");
}
