validation: Allow interpolate(flat) on integral IO

Produce a warning if the attribute is missing for integral vertex
outputs or fragment inputs. This will become an error in the future,
as per the WGSL spec.

Add the attribute to E2E tests.

Bug: tint:1224
Change-Id: Ia1f353f36cb7db516cf9e8b4877423dec3b3e711
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67160
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
James Price 2021-10-21 23:08:44 +00:00 committed by Tint LUCI CQ
parent 1aa98e62c0
commit a41694e9a3
33 changed files with 173 additions and 88 deletions

View File

@ -12,6 +12,7 @@
* `any()` and `all()` now support a `bool` parameter. These simply return the passed argument. [tint:1253](https://crbug.com/tint/1253)
* Call statements may now include functions that return a value (`ignore()` is no longer needed).
* The `interpolate(flat)` attribute can now be specified on integral user-defined IO. It will eventually become an error to define integral user-defined IO without this attribute.
### Fixes

View File

@ -1286,6 +1286,58 @@ TEST_P(InterpolateParameterTest, All) {
}
}
TEST_P(InterpolateParameterTest, IntegerScalar) {
auto& params = GetParam();
Func("main",
ast::VariableList{Param(
"a", ty.i32(),
{Location(0),
Interpolate(Source{{12, 34}}, params.type, params.sampling)})},
ty.void_(), {},
ast::DecorationList{Stage(ast::PipelineStage::kFragment)});
if (params.type != ast::InterpolationType::kFlat) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: interpolation type must be 'flat' for integral "
"user-defined IO types");
} else if (params.should_pass) {
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: flat interpolation attribute must not have a "
"sampling parameter");
}
}
TEST_P(InterpolateParameterTest, IntegerVector) {
auto& params = GetParam();
Func("main",
ast::VariableList{Param(
"a", ty.vec4<u32>(),
{Location(0),
Interpolate(Source{{12, 34}}, params.type, params.sampling)})},
ty.void_(), {},
ast::DecorationList{Stage(ast::PipelineStage::kFragment)});
if (params.type != ast::InterpolationType::kFlat) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: interpolation type must be 'flat' for integral "
"user-defined IO types");
} else if (params.should_pass) {
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: flat interpolation attribute must not have a "
"sampling parameter");
}
}
INSTANTIATE_TEST_SUITE_P(
ResolverDecorationValidationTest,
InterpolateParameterTest,
@ -1315,33 +1367,35 @@ INSTANTIATE_TEST_SUITE_P(
Params{ast::InterpolationType::kFlat,
ast::InterpolationSampling::kSample, false}));
TEST_F(InterpolateTest, Parameter_NotFloatingPoint) {
TEST_F(InterpolateTest, FragmentInput_Integer_MissingFlatInterpolation) {
Func("main",
ast::VariableList{
Param("a", ty.i32(),
{Location(0),
Interpolate(Source{{12, 34}}, ast::InterpolationType::kFlat,
ast::InterpolationSampling::kNone)})},
ast::VariableList{Param(Source{{12, 34}}, "a", ty.i32(), {Location(0)})},
ty.void_(), {},
ast::DecorationList{Stage(ast::PipelineStage::kFragment)});
EXPECT_FALSE(r()->Resolve());
// TODO(crbug.com/tint/1224): Make this an error.
EXPECT_TRUE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: store type of interpolate attribute must be floating "
"point scalar or vector");
"12:34 warning: integral user-defined fragment inputs must have a "
"flat interpolation attribute");
}
TEST_F(InterpolateTest, ReturnType_NotFloatingPoint) {
Func(
"main", {}, ty.i32(), {Return(1)},
ast::DecorationList{Stage(ast::PipelineStage::kFragment)},
{Location(0), Interpolate(Source{{12, 34}}, ast::InterpolationType::kFlat,
ast::InterpolationSampling::kNone)});
TEST_F(InterpolateTest, VertexOutput_Integer_MissingFlatInterpolation) {
auto* s = Structure(
"S",
{
Member("pos", ty.vec4<f32>(), {Builtin(ast::Builtin::kPosition)}),
Member(Source{{12, 34}}, "u", ty.u32(), {Location(0)}),
},
{});
Func("main", {}, ty.Of(s), {Return(Construct(ty.Of(s)))},
ast::DecorationList{Stage(ast::PipelineStage::kVertex)});
EXPECT_FALSE(r()->Resolve());
// TODO(crbug.com/tint/1224): Make this an error.
EXPECT_TRUE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: store type of interpolate attribute must be floating "
"point scalar or vector");
"12:34 warning: integral user-defined vertex outputs must have a "
"flat interpolation attribute");
}
} // namespace

View File

@ -764,8 +764,8 @@ TEST_F(LocationDecorationTests, Duplicate_input) {
// [[stage(fragment)]]
// fn main([[location(1)]] param_a : f32,
// [[location(1)]] param_b : f32) {}
auto* param_a = Param("param_a", ty.u32(), {Location(1)});
auto* param_b = Param("param_b", ty.u32(), {Location(Source{{12, 34}}, 1)});
auto* param_a = Param("param_a", ty.f32(), {Location(1)});
auto* param_b = Param("param_b", ty.f32(), {Location(Source{{12, 34}}, 1)});
Func(Source{{12, 34}}, "main", {param_a, param_b}, ty.void_(), {},
{Stage(ast::PipelineStage::kFragment)});

View File

@ -1389,10 +1389,10 @@ bool Resolver::ValidateInterpolateDecoration(
const sem::Type* storage_type) {
auto* type = storage_type->UnwrapRef();
if (!type->is_float_scalar_or_vector()) {
if (type->is_integer_scalar_or_vector() &&
deco->type != ast::InterpolationType::kFlat) {
AddError(
"store type of interpolate attribute must be floating point scalar or "
"vector",
"interpolation type must be 'flat' for integral user-defined IO types",
deco->source);
return false;
}
@ -1519,6 +1519,7 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func,
// Scan decorations for pipeline IO attributes.
// Check for overlap with attributes that have been seen previously.
const ast::Decoration* pipeline_io_attribute = nullptr;
const ast::InterpolateDecoration* interpolate_attribute = nullptr;
const ast::InvariantDecoration* invariant_attribute = nullptr;
for (auto* deco : decos) {
auto is_invalid_compute_shader_decoration = false;
@ -1566,6 +1567,7 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func,
} else if (!ValidateInterpolateDecoration(interpolate, ty)) {
return false;
}
interpolate_attribute = interpolate;
} else if (auto* invariant = deco->As<ast::InvariantDecoration>()) {
if (func->PipelineStage() == ast::PipelineStage::kCompute) {
is_invalid_compute_shader_decoration = true;
@ -1600,6 +1602,28 @@ bool Resolver::ValidateEntryPoint(const ast::Function* func,
return false;
}
if (pipeline_io_attribute &&
pipeline_io_attribute->Is<ast::LocationDecoration>()) {
if (ty->is_integer_scalar_or_vector() && !interpolate_attribute) {
// TODO(crbug.com/tint/1224): Make these errors once downstream
// usages have caught up (no sooner than M99).
if (func->PipelineStage() == ast::PipelineStage::kVertex &&
param_or_ret == ParamOrRetType::kReturnType) {
AddWarning(
"integral user-defined vertex outputs must have a flat "
"interpolation attribute",
source);
}
if (func->PipelineStage() == ast::PipelineStage::kFragment &&
param_or_ret == ParamOrRetType::kParameter) {
AddWarning(
"integral user-defined fragment inputs must have a flat "
"interpolation attribute",
source);
}
}
}
if (invariant_attribute) {
bool has_position = false;
if (pipeline_io_attribute) {

View File

@ -161,8 +161,11 @@ struct CanonicalizeEntryPointIO::State {
if (cfg.shader_style == ShaderStyle::kSpirv) {
// Vulkan requires that integer user-defined fragment inputs are
// always decorated with `Flat`.
// TODO(crbug.com/tint/1224): Remove this once a flat interpolation
// attribute is required for integers.
if (type->is_integer_scalar_or_vector() &&
ast::HasDecoration<ast::LocationDecoration>(attributes) &&
!ast::HasDecoration<ast::InterpolateDecoration>(attributes) &&
func_ast->PipelineStage() == ast::PipelineStage::kFragment) {
attributes.push_back(ctx.dst->Interpolate(
ast::InterpolationType::kFlat, ast::InterpolationSampling::kNone));
@ -217,9 +220,12 @@ struct CanonicalizeEntryPointIO::State {
const ast::Expression* value) {
// Vulkan requires that integer user-defined vertex outputs are
// always decorated with `Flat`.
// TODO(crbug.com/tint/1224): Remove this once a flat interpolation
// attribute is required for integers.
if (cfg.shader_style == ShaderStyle::kSpirv &&
type->is_integer_scalar_or_vector() &&
ast::HasDecoration<ast::LocationDecoration>(attributes) &&
!ast::HasDecoration<ast::InterpolateDecoration>(attributes) &&
func_ast->PipelineStage() == ast::PipelineStage::kVertex) {
attributes.push_back(ctx.dst->Interpolate(
ast::InterpolationType::kFlat, ast::InterpolationSampling::kNone));

View File

@ -1,7 +1,7 @@
[[stage(fragment)]]
fn main(
[[location(0)]] loc0 : i32,
[[location(1)]] loc1 : u32,
[[location(0), interpolate(flat)]] loc0 : i32,
[[location(1), interpolate(flat)]] loc1 : u32,
[[location(2)]] loc2 : f32,
[[location(3)]] loc3 : vec4<f32>,
) {

View File

@ -1,6 +1,6 @@
struct tint_symbol_1 {
int loc0 : TEXCOORD0;
uint loc1 : TEXCOORD1;
nointerpolation int loc0 : TEXCOORD0;
nointerpolation uint loc1 : TEXCOORD1;
float loc2 : TEXCOORD2;
float4 loc3 : TEXCOORD3;
};

View File

@ -2,8 +2,8 @@
using namespace metal;
struct tint_symbol_2 {
int loc0 [[user(locn0)]];
uint loc1 [[user(locn1)]];
int loc0 [[user(locn0)]] [[flat]];
uint loc1 [[user(locn1)]] [[flat]];
float loc2 [[user(locn2)]];
float4 loc3 [[user(locn3)]];
};

View File

@ -1,5 +1,5 @@
[[stage(fragment)]]
fn main([[location(0)]] loc0 : i32, [[location(1)]] loc1 : u32, [[location(2)]] loc2 : f32, [[location(3)]] loc3 : vec4<f32>) {
fn main([[location(0), interpolate(flat)]] loc0 : i32, [[location(1), interpolate(flat)]] loc1 : u32, [[location(2)]] loc2 : f32, [[location(3)]] loc3 : vec4<f32>) {
let i : i32 = loc0;
let u : u32 = loc1;
let f : f32 = loc2;

View File

@ -1,6 +1,6 @@
struct FragmentInputs {
[[location(0)]] loc0 : i32;
[[location(1)]] loc1 : u32;
[[location(0), interpolate(flat)]] loc0 : i32;
[[location(1), interpolate(flat)]] loc1 : u32;
[[location(2)]] loc2 : f32;
[[location(3)]] loc3 : vec4<f32>;
};

View File

@ -5,8 +5,8 @@ struct FragmentInputs {
float4 loc3;
};
struct tint_symbol_1 {
int loc0 : TEXCOORD0;
uint loc1 : TEXCOORD1;
nointerpolation int loc0 : TEXCOORD0;
nointerpolation uint loc1 : TEXCOORD1;
float loc2 : TEXCOORD2;
float4 loc3 : TEXCOORD3;
};

View File

@ -8,8 +8,8 @@ struct FragmentInputs {
float4 loc3;
};
struct tint_symbol_2 {
int loc0 [[user(locn0)]];
uint loc1 [[user(locn1)]];
int loc0 [[user(locn0)]] [[flat]];
uint loc1 [[user(locn1)]] [[flat]];
float loc2 [[user(locn2)]];
float4 loc3 [[user(locn3)]];
};

View File

@ -1,7 +1,7 @@
struct FragmentInputs {
[[location(0)]]
[[location(0), interpolate(flat)]]
loc0 : i32;
[[location(1)]]
[[location(1), interpolate(flat)]]
loc1 : u32;
[[location(2)]]
loc2 : f32;

View File

@ -1,6 +1,6 @@
struct FragmentInputs0 {
[[builtin(position)]] position : vec4<f32>;
[[location(0)]] loc0 : i32;
[[location(0), interpolate(flat)]] loc0 : i32;
};
struct FragmentInputs1 {
[[location(3)]] loc3 : vec4<f32>;
@ -11,7 +11,7 @@ struct FragmentInputs1 {
fn main(
inputs0 : FragmentInputs0,
[[builtin(front_facing)]] front_facing : bool,
[[location(1)]] loc1 : u32,
[[location(1), interpolate(flat)]] loc1 : u32,
[[builtin(sample_index)]] sample_index : u32,
inputs1 : FragmentInputs1,
[[location(2)]] loc2 : f32,

View File

@ -7,8 +7,8 @@ struct FragmentInputs1 {
uint sample_mask;
};
struct tint_symbol_1 {
int loc0 : TEXCOORD0;
uint loc1 : TEXCOORD1;
nointerpolation int loc0 : TEXCOORD0;
nointerpolation uint loc1 : TEXCOORD1;
float loc2 : TEXCOORD2;
float4 loc3 : TEXCOORD3;
float4 position : SV_Position;

View File

@ -10,8 +10,8 @@ struct FragmentInputs1 {
uint sample_mask;
};
struct tint_symbol_2 {
int loc0 [[user(locn0)]];
uint loc1 [[user(locn1)]];
int loc0 [[user(locn0)]] [[flat]];
uint loc1 [[user(locn1)]] [[flat]];
float loc2 [[user(locn2)]];
float4 loc3 [[user(locn3)]];
};

View File

@ -1,7 +1,7 @@
struct FragmentInputs0 {
[[builtin(position)]]
position : vec4<f32>;
[[location(0)]]
[[location(0), interpolate(flat)]]
loc0 : i32;
};
@ -13,7 +13,7 @@ struct FragmentInputs1 {
};
[[stage(fragment)]]
fn main(inputs0 : FragmentInputs0, [[builtin(front_facing)]] front_facing : bool, [[location(1)]] loc1 : u32, [[builtin(sample_index)]] sample_index : u32, inputs1 : FragmentInputs1, [[location(2)]] loc2 : f32) {
fn main(inputs0 : FragmentInputs0, [[builtin(front_facing)]] front_facing : bool, [[location(1), interpolate(flat)]] loc1 : u32, [[builtin(sample_index)]] sample_index : u32, inputs1 : FragmentInputs1, [[location(2)]] loc2 : f32) {
if (front_facing) {
let foo : vec4<f32> = inputs0.position;
let bar : u32 = (sample_index + inputs1.sample_mask);

View File

@ -1,8 +1,8 @@
struct Interface {
[[location(0)]] i : i32;
[[location(1)]] u : u32;
[[location(2)]] vi : vec4<i32>;
[[location(3)]] vu : vec4<u32>;
[[location(0), interpolate(flat)]] i : i32;
[[location(1), interpolate(flat)]] u : u32;
[[location(2), interpolate(flat)]] vi : vec4<i32>;
[[location(3), interpolate(flat)]] vu : vec4<u32>;
[[builtin(position)]] pos : vec4<f32>;
};

View File

@ -6,10 +6,10 @@ struct Interface {
float4 pos;
};
struct tint_symbol {
int i : TEXCOORD0;
uint u : TEXCOORD1;
int4 vi : TEXCOORD2;
uint4 vu : TEXCOORD3;
nointerpolation int i : TEXCOORD0;
nointerpolation uint u : TEXCOORD1;
nointerpolation int4 vi : TEXCOORD2;
nointerpolation uint4 vu : TEXCOORD3;
float4 pos : SV_Position;
};
@ -30,10 +30,10 @@ tint_symbol vert_main() {
}
struct tint_symbol_2 {
int i : TEXCOORD0;
uint u : TEXCOORD1;
int4 vi : TEXCOORD2;
uint4 vu : TEXCOORD3;
nointerpolation int i : TEXCOORD0;
nointerpolation uint u : TEXCOORD1;
nointerpolation int4 vi : TEXCOORD2;
nointerpolation uint4 vu : TEXCOORD3;
float4 pos : SV_Position;
};
struct tint_symbol_3 {

View File

@ -9,17 +9,17 @@ struct Interface {
float4 pos;
};
struct tint_symbol {
int i [[user(locn0)]];
uint u [[user(locn1)]];
int4 vi [[user(locn2)]];
uint4 vu [[user(locn3)]];
int i [[user(locn0)]] [[flat]];
uint u [[user(locn1)]] [[flat]];
int4 vi [[user(locn2)]] [[flat]];
uint4 vu [[user(locn3)]] [[flat]];
float4 pos [[position]];
};
struct tint_symbol_2 {
int i [[user(locn0)]];
uint u [[user(locn1)]];
int4 vi [[user(locn2)]];
uint4 vu [[user(locn3)]];
int i [[user(locn0)]] [[flat]];
uint u [[user(locn1)]] [[flat]];
int4 vi [[user(locn2)]] [[flat]];
uint4 vu [[user(locn3)]] [[flat]];
};
struct tint_symbol_3 {
int value [[color(0)]];

View File

@ -1,11 +1,11 @@
struct Interface {
[[location(0)]]
[[location(0), interpolate(flat)]]
i : i32;
[[location(1)]]
[[location(1), interpolate(flat)]]
u : u32;
[[location(2)]]
[[location(2), interpolate(flat)]]
vi : vec4<i32>;
[[location(3)]]
[[location(3), interpolate(flat)]]
vu : vec4<u32>;
[[builtin(position)]]
pos : vec4<f32>;

View File

@ -1,6 +1,6 @@
struct VertexOutput {
[[builtin(position)]] pos : vec4<f32>;
[[location(0)]] loc0 : i32;
[[location(0), interpolate(flat)]] loc0 : i32;
};
fn foo(x : f32) -> VertexOutput {

View File

@ -9,7 +9,7 @@ VertexOutput foo(float x) {
}
struct tint_symbol {
int loc0 : TEXCOORD0;
nointerpolation int loc0 : TEXCOORD0;
float4 pos : SV_Position;
};
@ -26,7 +26,7 @@ tint_symbol vert_main1() {
}
struct tint_symbol_1 {
int loc0 : TEXCOORD0;
nointerpolation int loc0 : TEXCOORD0;
float4 pos : SV_Position;
};

View File

@ -6,11 +6,11 @@ struct VertexOutput {
int loc0;
};
struct tint_symbol {
int loc0 [[user(locn0)]];
int loc0 [[user(locn0)]] [[flat]];
float4 pos [[position]];
};
struct tint_symbol_1 {
int loc0 [[user(locn0)]];
int loc0 [[user(locn0)]] [[flat]];
float4 pos [[position]];
};

View File

@ -1,7 +1,7 @@
struct VertexOutput {
[[builtin(position)]]
pos : vec4<f32>;
[[location(0)]]
[[location(0), interpolate(flat)]]
loc0 : i32;
};

View File

@ -1,7 +1,7 @@
[[block]]
struct S {
[[align(64)]] [[location(0)]] f : f32;
[[size(32)]] [[location(1)]] u : u32;
[[size(32)]] [[location(1), interpolate(flat)]] u : u32;
[[align(128)]] [[builtin(position)]] v : vec4<f32>;
};

View File

@ -8,7 +8,7 @@ RWByteAddressBuffer output : register(u0, space0);
struct tint_symbol_1 {
float f : TEXCOORD0;
uint u : TEXCOORD1;
nointerpolation uint u : TEXCOORD1;
float4 v : SV_Position;
};

View File

@ -10,7 +10,7 @@ struct S {
};
struct tint_symbol_1 {
float f [[user(locn0)]];
uint u [[user(locn1)]];
uint u [[user(locn1)]] [[flat]];
};
void frag_main_inner(device S& output, S input) {

View File

@ -2,7 +2,7 @@
struct S {
[[align(64), location(0)]]
f : f32;
[[size(32), location(1)]]
[[size(32), location(1), interpolate(flat)]]
u : u32;
[[align(128), builtin(position)]]
v : vec4<f32>;

View File

@ -1,6 +1,6 @@
struct VertexOutputs {
[[location(0)]] loc0 : i32;
[[location(1)]] loc1 : u32;
[[location(0), interpolate(flat)]] loc0 : i32;
[[location(1), interpolate(flat)]] loc1 : u32;
[[location(2)]] loc2 : f32;
[[location(3)]] loc3 : vec4<f32>;
[[builtin(position)]] position : vec4<f32>;

View File

@ -6,8 +6,8 @@ struct VertexOutputs {
float4 position;
};
struct tint_symbol {
int loc0 : TEXCOORD0;
uint loc1 : TEXCOORD1;
nointerpolation int loc0 : TEXCOORD0;
nointerpolation uint loc1 : TEXCOORD1;
float loc2 : TEXCOORD2;
float4 loc3 : TEXCOORD3;
float4 position : SV_Position;

View File

@ -9,8 +9,8 @@ struct VertexOutputs {
float4 position;
};
struct tint_symbol_1 {
int loc0 [[user(locn0)]];
uint loc1 [[user(locn1)]];
int loc0 [[user(locn0)]] [[flat]];
uint loc1 [[user(locn1)]] [[flat]];
float loc2 [[user(locn2)]];
float4 loc3 [[user(locn3)]];
float4 position [[position]];

View File

@ -1,7 +1,7 @@
struct VertexOutputs {
[[location(0)]]
[[location(0), interpolate(flat)]]
loc0 : i32;
[[location(1)]]
[[location(1), interpolate(flat)]]
loc1 : u32;
[[location(2)]]
loc2 : f32;