From d1469c60c1ab5c96278d570aa93ef38b722b67e4 Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 17 Dec 2020 16:29:31 +0000 Subject: [PATCH] spirv-reader: fix arity for depth texture builtins Texture buitins on depth textures always result in scalar f32. Corresponding operations in SPIR-V always result in vec4. When translating to Tint AST, wrap the generated texture builtin in a type constructor to vec4, with the builtin result as the x component. Fixed: tint:411 Change-Id: Idf26a1cbc7e875bc8a97bf3c0b342c0220c6d4b7 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35900 Auto-Submit: David Neto Commit-Queue: Ryan Harrison Reviewed-by: Ryan Harrison --- src/reader/spirv/function.cc | 23 ++- src/reader/spirv/parser_impl_handle_test.cc | 190 ++++++++++++-------- 2 files changed, 136 insertions(+), 77 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index f81cc77932..97907ab0e7 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -4145,14 +4145,31 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { // It returns a value. ast::Expression* value = call_expr; - // If necessary, convert the result to the signedness of the instruction - // result type. Compare the SPIR-V image's sampled component type with the - // component of the result type of the SPIR-V instruction. + // The result type, derived from the SPIR-V instruction. auto* result_type = parser_impl_.ConvertType(inst.type_id()); auto* result_component_type = result_type; if (auto* result_vector_type = result_type->As()) { result_component_type = result_vector_type->type(); } + + // Convert the arity of the result when operating on depth textures. + // SPIR-V operations on depth textures always result in 4-element vectors. + // But WGSL operations on depth textures always result in a f32 scalar. + // Construct a 4-element vector with the result from the builtin in the + // first component. + if (texture_type->Is()) { + value = create( + Source{}, + result_type, // a vec4 + ast::ExpressionList{ + value, parser_impl_.MakeNullValue(result_component_type), + parser_impl_.MakeNullValue(result_component_type), + parser_impl_.MakeNullValue(result_component_type)}); + } + + // If necessary, convert the result to the signedness of the instruction + // result type. Compare the SPIR-V image's sampled component type with the + // component of the result type of the SPIR-V instruction. auto* spirv_image_type = parser_impl_.GetSpirvTypeForHandleMemoryObjectDeclaration(*image); if (!spirv_image_type || (spirv_image_type->opcode() != SpvOpTypeImage)) { diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc index 4731bedc38..1cf0f1fb28 100644 --- a/src/reader/spirv/parser_impl_handle_test.cc +++ b/src/reader/spirv/parser_impl_handle_test.cc @@ -1751,13 +1751,19 @@ INSTANTIATE_TEST_SUITE_P( none __vec_4__f32 { - Call[not set]{ - Identifier[not set]{textureSample} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_10} - Identifier[not set]{coords12} - ) + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSample} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{coords12} + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} } } } @@ -1768,14 +1774,20 @@ INSTANTIATE_TEST_SUITE_P( none __vec_4__f32 { - Call[not set]{ - Identifier[not set]{textureSampleCompare} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_30} - Identifier[not set]{coords12} - ScalarConstructor[not set]{0.200000} - ) + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSampleCompare} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_30} + Identifier[not set]{coords12} + ScalarConstructor[not set]{0.200000} + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} } } } @@ -1809,14 +1821,20 @@ INSTANTIATE_TEST_SUITE_P( __depth_texture_2d })", R"( - Call[not set]{ - Identifier[not set]{textureSampleCompare} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_10} - Identifier[not set]{coords12} - ScalarConstructor[not set]{0.200000} - ) + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSampleCompare} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{coords12} + ScalarConstructor[not set]{0.200000} + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} })"}, // ImageSampleDrefImplicitLod - arrayed ImageAccessCase{"%float 2D 0 1 0 1 Unknown", @@ -1842,24 +1860,30 @@ INSTANTIATE_TEST_SUITE_P( __depth_texture_2d_array })", R"( - Call[not set]{ - Identifier[not set]{textureSampleCompare} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_10} - MemberAccessor[not set]{ - Identifier[not set]{coords123} - Identifier[not set]{xy} - } - TypeConstructor[not set]{ - __i32 + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSampleCompare} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} MemberAccessor[not set]{ Identifier[not set]{coords123} - Identifier[not set]{z} + Identifier[not set]{xy} } - } - ScalarConstructor[not set]{0.200000} - ) + TypeConstructor[not set]{ + __i32 + MemberAccessor[not set]{ + Identifier[not set]{coords123} + Identifier[not set]{z} + } + } + ScalarConstructor[not set]{0.200000} + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} })"}, // ImageSampleDrefImplicitLod with ConstOffset ImageAccessCase{ @@ -1886,15 +1910,21 @@ INSTANTIATE_TEST_SUITE_P( __depth_texture_2d })", R"( - Call[not set]{ - Identifier[not set]{textureSampleCompare} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_10} - Identifier[not set]{coords12} - ScalarConstructor[not set]{0.200000} - Identifier[not set]{offsets2d} - ) + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSampleCompare} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{coords12} + ScalarConstructor[not set]{0.200000} + Identifier[not set]{offsets2d} + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} })"}, // ImageSampleDrefImplicitLod arrayed with ConstOffset ImageAccessCase{ @@ -1921,25 +1951,31 @@ INSTANTIATE_TEST_SUITE_P( __depth_texture_2d_array })", R"( - Call[not set]{ - Identifier[not set]{textureSampleCompare} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_10} - MemberAccessor[not set]{ - Identifier[not set]{coords123} - Identifier[not set]{xy} - } - TypeConstructor[not set]{ - __i32 + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSampleCompare} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} MemberAccessor[not set]{ Identifier[not set]{coords123} - Identifier[not set]{z} + Identifier[not set]{xy} } - } - ScalarConstructor[not set]{0.200000} - Identifier[not set]{offsets2d} - ) + TypeConstructor[not set]{ + __i32 + MemberAccessor[not set]{ + Identifier[not set]{coords123} + Identifier[not set]{z} + } + } + ScalarConstructor[not set]{0.200000} + Identifier[not set]{offsets2d} + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} })"})); INSTANTIATE_TEST_SUITE_P( @@ -2471,17 +2507,23 @@ INSTANTIATE_TEST_SUITE_P( __depth_texture_2d })", R"( - Call[not set]{ - Identifier[not set]{textureSampleLevel} - ( - Identifier[not set]{x_20} - Identifier[not set]{x_10} - Identifier[not set]{vf12} - TypeConstructor[not set]{ - __i32 - Identifier[not set]{f1} - } - ) + TypeConstructor[not set]{ + __vec_4__f32 + Call[not set]{ + Identifier[not set]{textureSampleLevel} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{vf12} + TypeConstructor[not set]{ + __i32 + Identifier[not set]{f1} + } + ) + } + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} })"}})); using SpvParserTest_ImageAccessTest =