tint: Fix SPIR-V validation around interpolation decorations

SPIRV-Val has tightended up validation around input / output interpolation decorations.
This change ensures that the parser and writer do the right thing.

Change-Id: I29c97fdcc48c62aa77b106c42e64fbc54204d607
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/96020
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton 2022-07-12 19:10:39 +00:00 committed by Dawn LUCI CQ
parent c2eee95827
commit 51719ccc01
18 changed files with 68 additions and 222 deletions

View File

@ -1795,6 +1795,14 @@ bool ParserImpl::ConvertPipelineDecorations(const Type* store_type,
}
}
if (type == ast::InterpolationType::kFlat &&
!ast::HasAttribute<ast::LocationAttribute>(*attributes)) {
// WGSL requires that '@interpolate(flat)' needs to be paired with '@location', however
// SPIR-V requires all fragment shader integer Inputs are 'flat'. If the decorations do not
// contain a SpvDecorationLocation, then make this perspective.
type = ast::InterpolationType::kPerspective;
}
// Apply interpolation.
if (type == ast::InterpolationType::kPerspective &&
sampling == ast::InterpolationSampling::kNone) {

View File

@ -159,14 +159,16 @@ struct CanonicalizeEntryPointIO::State {
ast::AttributeList attributes) {
auto* ast_type = CreateASTTypeFor(ctx, type);
if (cfg.shader_style == ShaderStyle::kSpirv || cfg.shader_style == ShaderStyle::kGlsl) {
// 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::HasAttribute<ast::LocationAttribute>(attributes) &&
// Vulkan requires that integer user-defined fragment inputs are always decorated with
// `Flat`. See:
// https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/StandaloneSpirv.html#VUID-StandaloneSpirv-Flat-04744
// TODO(crbug.com/tint/1224): Remove this once a flat interpolation attribute is
// required for integers.
if (func_ast->PipelineStage() == ast::PipelineStage::kFragment &&
type->is_integer_scalar_or_vector() &&
!ast::HasAttribute<ast::InterpolateAttribute>(attributes) &&
func_ast->PipelineStage() == ast::PipelineStage::kFragment) {
(ast::HasAttribute<ast::LocationAttribute>(attributes) ||
cfg.shader_style == ShaderStyle::kSpirv)) {
attributes.push_back(ctx.dst->Interpolate(ast::InterpolationType::kFlat,
ast::InterpolationSampling::kNone));
}
@ -226,14 +228,15 @@ struct CanonicalizeEntryPointIO::State {
const sem::Type* type,
ast::AttributeList attributes,
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() &&
// 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 &&
func_ast->PipelineStage() == ast::PipelineStage::kVertex &&
type->is_integer_scalar_or_vector() &&
ast::HasAttribute<ast::LocationAttribute>(attributes) &&
!ast::HasAttribute<ast::InterpolateAttribute>(attributes) &&
func_ast->PipelineStage() == ast::PipelineStage::kVertex) {
!ast::HasAttribute<ast::InterpolateAttribute>(attributes)) {
attributes.push_back(ctx.dst->Interpolate(ast::InterpolationType::kFlat,
ast::InterpolationSampling::kNone));
}
@ -262,15 +265,19 @@ struct CanonicalizeEntryPointIO::State {
/// that will be passed to the original function.
/// @param param the original function parameter
void ProcessNonStructParameter(const sem::Parameter* param) {
// Do not add interpolation attributes on vertex input
bool do_interpolate = func_ast->PipelineStage() != ast::PipelineStage::kVertex;
// Remove the shader IO attributes from the inner function parameter, and
// attach them to the new object instead.
ast::AttributeList attributes;
for (auto* attr : param->Declaration()->attributes) {
if (IsShaderIOAttribute(attr)) {
ctx.Remove(param->Declaration()->attributes, attr);
if ((do_interpolate || !attr->Is<ast::InterpolateAttribute>())) {
attributes.push_back(ctx.Clone(attr));
}
}
}
auto name = ctx.src->Symbols().NameFor(param->Declaration()->symbol);
auto* input_expr = AddInput(name, param->Type(), std::move(attributes));
@ -283,6 +290,9 @@ struct CanonicalizeEntryPointIO::State {
/// the original function.
/// @param param the original function parameter
void ProcessStructParameter(const sem::Parameter* param) {
// Do not add interpolation attributes on vertex input
bool do_interpolate = func_ast->PipelineStage() != ast::PipelineStage::kVertex;
auto* str = param->Type()->As<sem::Struct>();
// Recreate struct members in the outer entry point and build an initializer
@ -297,12 +307,6 @@ struct CanonicalizeEntryPointIO::State {
auto* member_ast = member->Declaration();
auto name = ctx.src->Symbols().NameFor(member_ast->symbol);
// In GLSL, do not add interpolation attributes on vertex input
bool do_interpolate = true;
if (cfg.shader_style == ShaderStyle::kGlsl &&
func_ast->PipelineStage() == ast::PipelineStage::kVertex) {
do_interpolate = false;
}
auto attributes = CloneShaderIOAttributes(member_ast->attributes, do_interpolate);
auto* input_expr = AddInput(name, member->Type(), std::move(attributes));
inner_struct_values.push_back(input_expr);
@ -319,12 +323,8 @@ struct CanonicalizeEntryPointIO::State {
/// @param inner_ret_type the original function return type
/// @param original_result the result object produced by the original function
void ProcessReturnType(const sem::Type* inner_ret_type, Symbol original_result) {
bool do_interpolate = true;
// In GLSL, do not add interpolation attributes on fragment output
if (cfg.shader_style == ShaderStyle::kGlsl &&
func_ast->PipelineStage() == ast::PipelineStage::kFragment) {
do_interpolate = false;
}
// Do not add interpolation attributes on fragment output
bool do_interpolate = func_ast->PipelineStage() != ast::PipelineStage::kFragment;
if (auto* str = inner_ret_type->As<sem::Struct>()) {
for (auto* member : str->Members()) {
if (member->Type()->Is<sem::Struct>()) {

View File

@ -1978,13 +1978,13 @@ fn frag_main(inputs : FragmentInterface) -> FragmentInterface {
@location(3) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<in> vu_3 : vec4<u32>;
@location(0) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> i_4 : i32;
@location(0) @internal(disable_validation__ignore_storage_class) var<out> i_4 : i32;
@location(1) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> u_4 : u32;
@location(1) @internal(disable_validation__ignore_storage_class) var<out> u_4 : u32;
@location(2) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> vi_4 : vec4<i32>;
@location(2) @internal(disable_validation__ignore_storage_class) var<out> vi_4 : vec4<i32>;
@location(3) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> vu_4 : vec4<u32>;
@location(3) @internal(disable_validation__ignore_storage_class) var<out> vu_4 : vec4<u32>;
struct VertexIn {
i : i32,
@ -2108,13 +2108,13 @@ struct FragmentInterface {
@location(3) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<in> vu_3 : vec4<u32>;
@location(0) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> i_4 : i32;
@location(0) @internal(disable_validation__ignore_storage_class) var<out> i_4 : i32;
@location(1) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> u_4 : u32;
@location(1) @internal(disable_validation__ignore_storage_class) var<out> u_4 : u32;
@location(2) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> vi_4 : vec4<i32>;
@location(2) @internal(disable_validation__ignore_storage_class) var<out> vi_4 : vec4<i32>;
@location(3) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<out> vu_4 : vec4<u32>;
@location(3) @internal(disable_validation__ignore_storage_class) var<out> vu_4 : vec4<u32>;
fn vert_main_inner(in : VertexIn) -> VertexOut {
return VertexOut(in.i, in.u, in.vi, in.vu, vec4<f32>());
@ -2344,7 +2344,7 @@ struct tint_symbol_1 {
}
struct tint_symbol_2 {
@location(1) @interpolate(flat)
@location(1)
value : f32,
}
@ -2397,7 +2397,7 @@ struct tint_symbol_1 {
}
struct tint_symbol_2 {
@location(1) @interpolate(flat)
@location(1)
value : f32,
}
@ -3868,9 +3868,9 @@ fn main(@builtin(sample_index) sample_index : u32,
)";
auto* expect = R"(
@builtin(sample_index) @internal(disable_validation__ignore_storage_class) var<in> sample_index_1 : u32;
@builtin(sample_index) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<in> sample_index_1 : u32;
@builtin(sample_mask) @internal(disable_validation__ignore_storage_class) var<in> mask_in_1 : array<u32, 1u>;
@builtin(sample_mask) @interpolate(flat) @internal(disable_validation__ignore_storage_class) var<in> mask_in_1 : array<u32, 1u>;
@builtin(sample_mask) @internal(disable_validation__ignore_storage_class) var<out> value : array<u32, 1u>;

View File

@ -311,9 +311,12 @@ TEST_F(BuilderTest, SampleIndex_SampleRateShadingCapability) {
// Make sure we generate the SampleRateShading capability.
EXPECT_EQ(DumpInstructions(b.capabilities()),
"OpCapability Shader\n"
"OpCapability SampleRateShading\n");
EXPECT_EQ(DumpInstructions(b.annots()), "OpDecorate %1 BuiltIn SampleId\n");
R"(OpCapability Shader
OpCapability SampleRateShading
)");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 BuiltIn SampleId
OpDecorate %1 Flat
)");
}
} // namespace

View File

@ -22,6 +22,7 @@
OpDecorate %a_1 Location 0
OpDecorate %_arr_uint_uint_1 ArrayStride 4
OpDecorate %mask_1 BuiltIn SampleMask
OpDecorate %mask_1 Flat
OpDecorate %b_1 Location 1
OpDecorate %a_2 Location 0
OpDecorate %mask_2 BuiltIn SampleMask

View File

@ -21,8 +21,10 @@
OpDecorate %position_1 BuiltIn FragCoord
OpDecorate %front_facing_1 BuiltIn FrontFacing
OpDecorate %sample_index_1 BuiltIn SampleId
OpDecorate %sample_index_1 Flat
OpDecorate %_arr_uint_uint_1 ArrayStride 4
OpDecorate %sample_mask_1 BuiltIn SampleMask
OpDecorate %sample_mask_1 Flat
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Input_v4float = OpTypePointer Input %v4float

View File

@ -23,8 +23,10 @@
OpDecorate %position_1 BuiltIn FragCoord
OpDecorate %front_facing_1 BuiltIn FrontFacing
OpDecorate %sample_index_1 BuiltIn SampleId
OpDecorate %sample_index_1 Flat
OpDecorate %_arr_uint_uint_1 ArrayStride 4
OpDecorate %sample_mask_1 BuiltIn SampleMask
OpDecorate %sample_mask_1 Flat
OpMemberDecorate %FragmentInputs 0 Offset 0
OpMemberDecorate %FragmentInputs 1 Offset 16
OpMemberDecorate %FragmentInputs 2 Offset 20

View File

@ -37,9 +37,11 @@
OpDecorate %loc1_1 Location 1
OpDecorate %loc1_1 Flat
OpDecorate %sample_index_1 BuiltIn SampleId
OpDecorate %sample_index_1 Flat
OpDecorate %loc3_1 Location 3
OpDecorate %_arr_uint_uint_1 ArrayStride 4
OpDecorate %sample_mask_1 BuiltIn SampleMask
OpDecorate %sample_mask_1 Flat
OpDecorate %loc2_1 Location 2
OpMemberDecorate %FragmentInputs0 0 Offset 0
OpMemberDecorate %FragmentInputs0 1 Offset 16

View File

@ -10,8 +10,10 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %5 "main" %1 %2 %3 %4
OpExecutionMode %5 OriginUpperLeft
OpDecorate %1 Location 0
OpDecorate %1 Flat
OpDecorate %2 Location 0
OpDecorate %3 Location 30
OpDecorate %3 Flat
OpDecorate %4 Location 6
%void = OpTypeVoid
%7 = OpTypeFunction %void

View File

@ -1,67 +0,0 @@
; Test: SpvModuleScopeVarParserTest_EntryPointWrapping_Interpolation_Flat_Vertex_In.spvasm
; SPIR-V
; Version: 1.0
; Generator: Khronos SPIR-V Tools Assembler; 0
; Bound: 42
; Schema: 0
OpCapability Shader
OpCapability SampleRateShading
OpMemoryModel Logical Simple
OpEntryPoint Vertex %7 "main" %1 %2 %3 %4 %5 %6 %gl_Position
OpDecorate %1 Location 1
OpDecorate %2 Location 2
OpDecorate %3 Location 3
OpDecorate %4 Location 4
OpDecorate %5 Location 5
OpDecorate %6 Location 6
OpDecorate %1 Flat
OpDecorate %2 Flat
OpDecorate %3 Flat
OpDecorate %4 Flat
OpDecorate %5 Flat
OpDecorate %6 Flat
OpDecorate %gl_Position BuiltIn Position
%void = OpTypeVoid
%9 = OpTypeFunction %void
%bool = OpTypeBool
%float = OpTypeFloat 32
%uint = OpTypeInt 32 0
%int = OpTypeInt 32 1
%_ptr_Private_bool = OpTypePointer Private %bool
%_ptr_Private_float = OpTypePointer Private %float
%_ptr_Private_uint = OpTypePointer Private %uint
%_ptr_Private_int = OpTypePointer Private %int
%true = OpConstantTrue %bool
%false = OpConstantFalse %bool
%float_0 = OpConstant %float 0
%float_1_5 = OpConstant %float 1.5
%uint_1 = OpConstant %uint 1
%int_n1 = OpConstant %int -1
%int_14 = OpConstant %int 14
%uint_2 = OpConstant %uint 2
%v2bool = OpTypeVector %bool 2
%v2uint = OpTypeVector %uint 2
%v2int = OpTypeVector %int 2
%v2float = OpTypeVector %float 2
%v4float = OpTypeVector %float 4
%mat3v2float = OpTypeMatrix %v2float 3
%_arr_uint_uint_2 = OpTypeArray %uint %uint_2
%_ptr_Input_uint = OpTypePointer Input %uint
%_ptr_Input_v2uint = OpTypePointer Input %v2uint
%_ptr_Input_int = OpTypePointer Input %int
%_ptr_Input_v2int = OpTypePointer Input %v2int
%_ptr_Input_float = OpTypePointer Input %float
%_ptr_Input_v2float = OpTypePointer Input %v2float
%1 = OpVariable %_ptr_Input_uint Input
%2 = OpVariable %_ptr_Input_v2uint Input
%3 = OpVariable %_ptr_Input_int Input
%4 = OpVariable %_ptr_Input_v2int Input
%5 = OpVariable %_ptr_Input_float Input
%6 = OpVariable %_ptr_Input_v2float Input
%_ptr_Output_v4float = OpTypePointer Output %v4float
%gl_Position = OpVariable %_ptr_Output_v4float Output
%7 = OpFunction %void None %9
%41 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -1,52 +0,0 @@
SKIP: FAILED
#version 310 es
layout(location = 1) flat in uint x_1_param_1;
layout(location = 2) flat in uvec2 x_2_param_1;
layout(location = 3) flat in int x_3_param_1;
layout(location = 4) flat in ivec2 x_4_param_1;
layout(location = 5) flat in float x_5_param_1;
layout(location = 6) flat in vec2 x_6_param_1;
uint x_1 = 0u;
uvec2 x_2 = uvec2(0u, 0u);
int x_3 = 0;
ivec2 x_4 = ivec2(0, 0);
float x_5 = 0.0f;
vec2 x_6 = vec2(0.0f, 0.0f);
vec4 x_8 = vec4(0.0f, 0.0f, 0.0f, 0.0f);
void main_1() {
return;
}
struct main_out {
vec4 x_8_1;
};
main_out tint_symbol(uint x_1_param, uvec2 x_2_param, int x_3_param, ivec2 x_4_param, float x_5_param, vec2 x_6_param) {
x_1 = x_1_param;
x_2 = x_2_param;
x_3 = x_3_param;
x_4 = x_4_param;
x_5 = x_5_param;
x_6 = x_6_param;
main_1();
main_out tint_symbol_1 = main_out(x_8);
return tint_symbol_1;
}
void main() {
gl_PointSize = 1.0;
main_out inner_result = tint_symbol(x_1_param_1, x_2_param_1, x_3_param_1, x_4_param_1, x_5_param_1, x_6_param_1);
gl_Position = inner_result.x_8_1;
gl_Position.y = -(gl_Position.y);
gl_Position.z = ((2.0f * gl_Position.z) - gl_Position.w);
return;
}
Error parsing GLSL shader:
ERROR: 0:3: '' : vertex input cannot be further qualified
ERROR: 0:3: '' : compilation terminated
ERROR: 2 compilation errors. No code generated.

View File

@ -1,61 +0,0 @@
; Test: SpvModuleScopeVarParserTest_EntryPointWrapping_Interpolation_Floating_Fragment_Out.spvasm
; SPIR-V
; Version: 1.0
; Generator: Khronos SPIR-V Tools Assembler; 0
; Bound: 35
; Schema: 0
OpCapability Shader
OpCapability SampleRateShading
OpMemoryModel Logical Simple
OpEntryPoint Fragment %7 "main" %1 %2 %3 %4 %5 %6
OpExecutionMode %7 OriginUpperLeft
OpDecorate %1 Location 1
OpDecorate %2 Location 2
OpDecorate %3 Location 3
OpDecorate %4 Location 4
OpDecorate %5 Location 5
OpDecorate %6 Location 6
OpDecorate %2 Centroid
OpDecorate %3 Sample
OpDecorate %4 NoPerspective
OpDecorate %5 NoPerspective
OpDecorate %5 Centroid
OpDecorate %6 NoPerspective
OpDecorate %6 Sample
%void = OpTypeVoid
%9 = OpTypeFunction %void
%bool = OpTypeBool
%float = OpTypeFloat 32
%uint = OpTypeInt 32 0
%int = OpTypeInt 32 1
%_ptr_Private_bool = OpTypePointer Private %bool
%_ptr_Private_float = OpTypePointer Private %float
%_ptr_Private_uint = OpTypePointer Private %uint
%_ptr_Private_int = OpTypePointer Private %int
%true = OpConstantTrue %bool
%false = OpConstantFalse %bool
%float_0 = OpConstant %float 0
%float_1_5 = OpConstant %float 1.5
%uint_1 = OpConstant %uint 1
%int_n1 = OpConstant %int -1
%int_14 = OpConstant %int 14
%uint_2 = OpConstant %uint 2
%v2bool = OpTypeVector %bool 2
%v2uint = OpTypeVector %uint 2
%v2int = OpTypeVector %int 2
%v2float = OpTypeVector %float 2
%v4float = OpTypeVector %float 4
%mat3v2float = OpTypeMatrix %v2float 3
%_arr_uint_uint_2 = OpTypeArray %uint %uint_2
%_ptr_Output_float = OpTypePointer Output %float
%1 = OpVariable %_ptr_Output_float Output
%2 = OpVariable %_ptr_Output_float Output
%3 = OpVariable %_ptr_Output_float Output
%4 = OpVariable %_ptr_Output_float Output
%5 = OpVariable %_ptr_Output_float Output
%6 = OpVariable %_ptr_Output_float Output
%7 = OpFunction %void None %9
%34 = OpLabel
OpReturn
OpFunctionEnd

View File

@ -10,6 +10,7 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %3 "main" %gl_SampleID
OpExecutionMode %3 OriginUpperLeft
OpDecorate %gl_SampleID BuiltIn SampleId
OpDecorate %gl_SampleID Flat
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32

View File

@ -10,6 +10,7 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %3 "main" %gl_SampleID
OpExecutionMode %3 OriginUpperLeft
OpDecorate %gl_SampleID BuiltIn SampleId
OpDecorate %gl_SampleID Flat
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32

View File

@ -10,6 +10,7 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %3 "main" %gl_SampleID
OpExecutionMode %3 OriginUpperLeft
OpDecorate %gl_SampleID BuiltIn SampleId
OpDecorate %gl_SampleID Flat
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32

View File

@ -10,6 +10,7 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %3 "main" %gl_SampleID
OpExecutionMode %3 OriginUpperLeft
OpDecorate %gl_SampleID BuiltIn SampleId
OpDecorate %gl_SampleID Flat
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32

View File

@ -10,6 +10,7 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %3 "main" %gl_SampleID
OpExecutionMode %3 OriginUpperLeft
OpDecorate %gl_SampleID BuiltIn SampleId
OpDecorate %gl_SampleID Flat
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32

View File

@ -10,6 +10,7 @@ OpMemoryModel Logical Simple
OpEntryPoint Fragment %3 "main" %gl_SampleID
OpExecutionMode %3 OriginUpperLeft
OpDecorate %gl_SampleID BuiltIn SampleId
OpDecorate %gl_SampleID Flat
%void = OpTypeVoid
%5 = OpTypeFunction %void
%float = OpTypeFloat 32