From 4beeaea9daf21186fabdde3fa7c24df0f1328309 Mon Sep 17 00:00:00 2001 From: David Neto Date: Tue, 4 Jan 2022 22:00:59 +0000 Subject: [PATCH] spirv-reader: support OpImageDrefGather Also, issue an error when a gather or dref-gather operation is used with a Bias or Grad image operand. Fixed: tint:1336 Change-Id: Ife11d2f52a1a2d1b75e26269373db5cc4b3440bf Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/74801 Reviewed-by: Ben Clayton Kokoro: Kokoro Commit-Queue: David Neto Auto-Submit: David Neto --- src/reader/spirv/function.cc | 50 +++++-- src/reader/spirv/parser_impl_handle_test.cc | 145 +++++++++++++++++--- 2 files changed, 163 insertions(+), 32 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index a51cc24dd8..45b622df09 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -5286,9 +5286,24 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { const auto num_args = inst.NumInOperands(); + // Consumes the depth-reference argument, pushing it onto the end of + // the parameter list. Issues a diagnostic and returns false on error. + auto consume_dref = [&]() -> bool { + if (arg_index < num_args) { + params.push_back(MakeOperand(inst, arg_index).expr); + arg_index++; + } else { + return Fail() + << "image depth-compare instruction is missing a Dref operand: " + << inst.PrettyPrint(); + } + return true; + }; + std::string builtin_name; bool use_level_of_detail_suffix = true; bool is_dref_sample = false; + bool is_gather_or_dref_gather = false; bool is_non_dref_sample = false; switch (opcode) { case SpvOpImageSampleImplicitLod: @@ -5304,16 +5319,12 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { case SpvOpImageSampleProjDrefExplicitLod: is_dref_sample = true; builtin_name = "textureSampleCompare"; - if (arg_index < num_args) { - params.push_back(MakeOperand(inst, arg_index).expr); - arg_index++; - } else { - return Fail() - << "image depth-compare instruction is missing a Dref operand: " - << inst.PrettyPrint(); + if (!consume_dref()) { + return false; } break; case SpvOpImageGather: + is_gather_or_dref_gather = true; builtin_name = "textureGather"; if (!texture_type->Is()) { // The explicit component is the *first* argument in WGSL. @@ -5323,7 +5334,12 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { arg_index++; break; case SpvOpImageDrefGather: - return Fail() << " image dref gather is not yet supported"; + is_gather_or_dref_gather = true; + builtin_name = "textureGatherCompare"; + if (!consume_dref()) { + return false; + } + break; case SpvOpImageFetch: case SpvOpImageRead: // Read a single texel from a sampled or storage image. @@ -5367,6 +5383,11 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { "level-of-detail bias: " << inst.PrettyPrint(); } + if (is_gather_or_dref_gather) { + return Fail() << "WGSL does not support image gather with " + "level-of-detail bias: " + << inst.PrettyPrint(); + } builtin_name += "Bias"; params.push_back(MakeOperand(inst, arg_index).expr); image_operands_mask ^= SpvImageOperandsBiasMask; @@ -5376,9 +5397,11 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { if (use_level_of_detail_suffix) { builtin_name += "Level"; } - if (is_dref_sample) { + if (is_dref_sample || is_gather_or_dref_gather) { // Metal only supports Lod = 0 for comparison sampling without // derivatives. + // Vulkan SPIR-V does not allow Lod with OpImageGather or + // OpImageDrefGather. if (!IsFloatZero(inst.GetSingleWordInOperand(arg_index))) { return Fail() << "WGSL comparison sampling without derivatives " "requires level-of-detail 0.0" @@ -5412,6 +5435,11 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { "explicit gradient: " << inst.PrettyPrint(); } + if (is_gather_or_dref_gather) { + return Fail() << "WGSL does not support image gather with " + "explicit gradient: " + << inst.PrettyPrint(); + } builtin_name += "Grad"; params.push_back(MakeOperand(inst, arg_index).expr); params.push_back(MakeOperand(inst, arg_index + 1).expr); @@ -5475,8 +5503,8 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { // compare sample f32 DrefImplicitLod f32 // compare sample f32 DrefExplicitLod f32 // texel load vec4 ImageFetch f32 - // normal gather vec4 ImageGather vec4 TODO(dneto) - // dref gather vec4 ImageFetch vec4 TODO(dneto) + // normal gather vec4 ImageGather vec4 + // dref gather vec4 ImageDrefGather vec4 // Construct a 4-element vector with the result from the builtin in the // first component. if (texture_type->IsAnyOf()) { diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc index 5d99c184c6..ef2eeb053f 100644 --- a/src/reader/spirv/parser_impl_handle_test.cc +++ b/src/reader/spirv/parser_impl_handle_test.cc @@ -929,12 +929,6 @@ TEST_P(SpvParserHandleTest_RegisterHandleUsage_SampledImage, Variable) { EXPECT_THAT(su.to_str(), Eq(GetParam().expected_sampler_usage)); EXPECT_THAT(iu.to_str(), Eq(GetParam().expected_image_usage)); - // TODO(dneto): remove this. crbug.com/tint/1336 - if (inst.find("Gather") != std::string::npos) { - // WGSL does not support Gather instructions yet. - // So don't emit them as part of a "passing" corpus. - p->DeliberatelyInvalidSpirv(); - } if (inst.find("ImageQueryLod") != std::string::npos) { // WGSL does not support querying image level of detail. // So don't emit them as part of a "passing" corpus. @@ -987,12 +981,6 @@ TEST_P(SpvParserHandleTest_RegisterHandleUsage_SampledImage, FunctionParam) { EXPECT_THAT(su.to_str(), Eq(GetParam().expected_sampler_usage)); EXPECT_THAT(iu.to_str(), Eq(GetParam().expected_image_usage)); - // TODO(dneto): remove this. crbug.com/tint/1336 - if (inst.find("Gather") != std::string::npos) { - // WGSL does not support Gather instructions yet. - // So don't emit them as part of a "passing" corpus. - p->DeliberatelyInvalidSpirv(); - } if (inst.find("ImageQueryLod") != std::string::npos) { // WGSL does not support querying image level of detail. // So don't emit them as part of a "passing" corpus. @@ -1721,15 +1709,87 @@ INSTANTIATE_TEST_SUITE_P( ImageDrefGather, SpvParserHandleTest_SampledImageAccessTest, ::testing::ValuesIn(std::vector{ - // TODO(dneto): OpImageDrefGather 2DDepth - // TODO(dneto): OpImageDrefGather 2DDepth ConstOffset signed - // TODO(dneto): OpImageDrefGather 2DDepth ConstOffset unsigned - // TODO(dneto): OpImageDrefGather 2DDepth Array - // TODO(dneto): OpImageDrefGather 2DDepth Array ConstOffset signed - // TODO(dneto): OpImageDrefGather 2DDepth Array ConstOffset unsigned - // TODO(dneto): OpImageDrefGather DepthCube - // TODO(dneto): OpImageDrefGather DepthCube Array - })); + // OpImageDrefGather 2DDepth + ImageAccessCase{ + "%float 2D 1 0 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords12 %depth", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_2d;)", + "textureGatherCompare(x_20, x_10, coords12, 0.200000003)"}, + // OpImageDrefGather 2DDepth ConstOffset signed + ImageAccessCase{ + "%float 2D 1 0 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords12 %depth ConstOffset %offsets2d", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_2d;)", + "textureGatherCompare(x_20, x_10, coords12, 0.200000003, " + "vec2(3, 4))"}, + // OpImageDrefGather 2DDepth ConstOffset unsigned + ImageAccessCase{ + "%float 2D 1 0 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords12 %depth ConstOffset " + "%u_offsets2d", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_2d;)", + "textureGatherCompare(x_20, x_10, coords12, 0.200000003, " + "vec2(vec2(3u, 4u)))"}, + // OpImageDrefGather 2DDepth Array + ImageAccessCase{ + "%float 2D 1 1 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords123 %depth", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_2d_array;)", + "textureGatherCompare(x_20, x_10, coords123.xy, " + "i32(round(coords123.z)), 0.200000003)"}, + // OpImageDrefGather 2DDepth Array ConstOffset signed + ImageAccessCase{ + "%float 2D 1 1 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords123 %depth ConstOffset %offsets2d", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_2d_array;)", + "textureGatherCompare(x_20, x_10, coords123.xy, " + "i32(round(coords123.z)), 0.200000003, vec2(3, 4))"}, + // OpImageDrefGather 2DDepth Array ConstOffset unsigned + ImageAccessCase{ + "%float 2D 1 1 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords123 %depth ConstOffset " + "%u_offsets2d", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_2d_array;)", + "textureGatherCompare(x_20, x_10, coords123.xy, " + "i32(round(coords123.z)), 0.200000003, " + "vec2(vec2(3u, 4u)))"}, + // OpImageDrefGather DepthCube + ImageAccessCase{ + "%float Cube 1 0 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords123 %depth", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_cube;)", + "textureGatherCompare(x_20, x_10, coords123, 0.200000003)"}, + // OpImageDrefGather DepthCube Array + ImageAccessCase{ + "%float Cube 1 1 0 1 Unknown", + "%result = OpImageDrefGather " + "%v4float %sampled_image %coords1234 %depth", + R"([[group(0), binding(0)]] var x_10 : sampler_comparison; + +[[group(2), binding(1)]] var x_20 : texture_depth_cube_array;)", + "textureGatherCompare(x_20, x_10, coords1234.xyz, " + "i32(round(coords1234.w)), 0.200000003)"}})); INSTANTIATE_TEST_SUITE_P( ImageSampleImplicitLod, @@ -3776,6 +3836,49 @@ INSTANTIATE_TEST_SUITE_P( "WGSL does not support querying the level of detail of an image: ", {}}})); +INSTANTIATE_TEST_SUITE_P( + ImageGather_Bias_IsError, + SpvParserHandleTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + {"%float 2D 0 0 0 1 Unknown", + "%result = OpImageGather %v4float %sampled_image %vf12 %int_1 " + "Bias %float_null", + "WGSL does not support image gather with level-of-detail bias: ", + {}}})); + +INSTANTIATE_TEST_SUITE_P( + ImageDrefGather_Bias_IsError, + SpvParserHandleTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + {"%float 2D 1 0 0 1 Unknown", + "%result = OpImageDrefGather %v4float %sampled_image %vf12 %depth " + "Bias %float_null", + "WGSL does not support image gather with level-of-detail bias: ", + {}}})); + +// Note: Vulkan SPIR-V ImageGather and ImageDrefGather do not allow explicit +// Lod. The SPIR-V validator should reject those cases already. + +INSTANTIATE_TEST_SUITE_P( + ImageGather_Grad_IsError, + SpvParserHandleTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + {"%float 2D 0 0 0 1 Unknown", + "%result = OpImageGather %v4float %sampled_image %vf12 %int_1 " + "Grad %vf12 %vf12", + "WGSL does not support image gather with explicit gradient: ", + {}}})); + +INSTANTIATE_TEST_SUITE_P( + ImageDrefGather_Grad_IsError, + SpvParserHandleTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + {"%float 2D 1 0 0 1 Unknown", + "%result = OpImageDrefGather %v4float %sampled_image %vf12 %depth " + "Grad %vf12 %vf12", + "WGSL does not support image gather with explicit gradient: ", + {}}})); + TEST_F(SpvParserHandleTest, NeverGenerateConstDeclForHandle_UseVariableDirectly) { // An ad-hoc test to prove we never had the issue