From 9d555d16217babdc93651a3f67c1c3a9cf3bb5c6 Mon Sep 17 00:00:00 2001 From: David Neto Date: Fri, 21 May 2021 19:17:33 +0000 Subject: [PATCH] spirv-reader: support textureSampleCompareLevel - Translates from OpImageSampleDrefExplicitLod, but the Lod must be a constant 0, or the reader issues an error. The requirement for Lod 0 is a constraint from Metal, inherited by WGSL. Fixed: tint:425, tint:482 Change-Id: I8fdf10dbd9c5ac3e24398519a28202c84677c38d Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51803 Commit-Queue: David Neto Auto-Submit: David Neto Reviewed-by: Ben Clayton --- src/reader/spirv/function.cc | 59 +++- src/reader/spirv/function.h | 5 + src/reader/spirv/parser_impl_handle_test.cc | 309 +++++++++++++++++++- 3 files changed, 345 insertions(+), 28 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index c3d6b062b2..0987f536de 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -3262,15 +3262,9 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { // Handle exceptional cases switch (GetSkipReason(ptr_id)) { case SkipReason::kPointSizeBuiltinPointer: - if (const auto* c = constant_mgr_->FindDeclaredConstant(value_id)) { - // If we're writing a constant 1.0, then skip the write. That's all - // that WebGPU handles. - auto* ct = c->type(); - if (ct->AsFloat() && (ct->AsFloat()->width() == 32) && - (c->GetFloat() == 1.0f)) { - // Don't store to PointSize - return true; - } + if (IsFloatOne(value_id)) { + // Don't store to PointSize + return true; } return Fail() << "cannot store a value other than constant 1.0 to " "PointSize builtin: " @@ -4843,13 +4837,26 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { if (use_level_of_detail_suffix) { builtin_name += "Level"; } - TypedExpression lod = MakeOperand(inst, arg_index); - // When sampling from a depth texture, the Lod operand must be an I32. - if (texture_type->Is()) { - // Convert it to a signed integer type. - lod = ToI32(lod); + if (is_dref_sample) { + // Metal only supports Lod = 0 for comparison sampling without + // derivatives. + if (!IsFloatZero(inst.GetSingleWordInOperand(arg_index))) { + return Fail() << "WGSL comparison sampling without derivatives " + "requires level-of-detail 0.0" + << inst.PrettyPrint(); + } + // Don't generate the Lod argument. + } else { + // Generate the Lod argument. + TypedExpression lod = MakeOperand(inst, arg_index); + // When sampling from a depth texture, the Lod operand must be an I32. + if (texture_type->Is()) { + // Convert it to a signed integer type. + lod = ToI32(lod); + } + params.push_back(lod.expr); } - params.push_back(lod.expr); + image_operands_mask ^= SpvImageOperandsLodMask; arg_index++; } else if ((opcode == SpvOpImageFetch) && @@ -5466,6 +5473,28 @@ TypedExpression FunctionEmitter::Dereference(TypedExpression expr) { }; } +bool FunctionEmitter::IsFloatZero(uint32_t value_id) { + if (const auto* c = constant_mgr_->FindDeclaredConstant(value_id)) { + if (const auto* float_const = c->AsFloatConstant()) { + return 0.0f == float_const->GetFloatValue(); + } + if (c->AsNullConstant()) { + // Valid SPIR-V requires it to be a float value anyway. + return true; + } + } + return false; +} + +bool FunctionEmitter::IsFloatOne(uint32_t value_id) { + if (const auto* c = constant_mgr_->FindDeclaredConstant(value_id)) { + if (const auto* float_const = c->AsFloatConstant()) { + return 1.0f == float_const->GetFloatValue(); + } + } + return false; +} + FunctionEmitter::FunctionDeclaration::FunctionDeclaration() = default; FunctionEmitter::FunctionDeclaration::~FunctionDeclaration() = default; diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index 940e84a312..d9d8384447 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -851,6 +851,11 @@ class FunctionEmitter { /// @returns the value itself, or converted to signed integral TypedExpression ToSignedIfUnsigned(TypedExpression value); + /// Returns true if the given SPIR-V id represents a constant float 0. + bool IsFloatZero(uint32_t value_id); + /// Returns true if the given SPIR-V id represents a constant float 1. + bool IsFloatOne(uint32_t value_id); + private: /// FunctionDeclaration contains the parsed information for a function header. struct FunctionDeclaration { diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc index 296bc5cde1..8569bb50fb 100644 --- a/src/reader/spirv/parser_impl_handle_test.cc +++ b/src/reader/spirv/parser_impl_handle_test.cc @@ -90,6 +90,7 @@ std::string CommonBasicTypes() { %v4float = OpTypeVector %float 4 %float_null = OpConstantNull %float + %float_0 = OpConstant %float 0 %float_1 = OpConstant %float 1 %float_2 = OpConstant %float 2 %float_3 = OpConstant %float 3 @@ -932,8 +933,6 @@ TEST_P(SpvParserHandleTest_RegisterHandleUsage_SampledImage, Variable) { p->DeliberatelyInvalidSpirv(); } if (inst.find("ImageSampleDrefExplicitLod") != std::string::npos) { - // WGSL does not support querying image level of detail. - // So don't emit them as part of a "passing" corpus. p->SkipDumpingPending("crbug.com/tint/425"); // gpuweb issue #1319 } } @@ -2259,6 +2258,253 @@ INSTANTIATE_TEST_SUITE_P( ) })"})); +INSTANTIATE_TEST_SUITE_P( + ImageSampleDrefExplicitLod, + SpvParserHandleTest_SampledImageAccessTest, + // Lod must be float constant 0 due to a Metal constraint. + // Another test checks cases where the Lod is not float constant 0. + ::testing::Values( + // 2D + ImageAccessCase{"%float 2D 1 0 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod " + "%float %sampled_image %coords12 %depth Lod %float_0", + R"( + Variable{ + Decorations{ + GroupDecoration{0} + BindingDecoration{0} + } + x_10 + none + __sampler_comparison + } + Variable{ + Decorations{ + GroupDecoration{2} + BindingDecoration{1} + } + x_20 + none + __depth_texture_2d + })", + R"( + Call[not set]{ + Identifier[not set]{textureSampleCompareLevel} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{coords12} + ScalarConstructor[not set]{0.200000} + ) + })"}, + // 2D array + ImageAccessCase{"%float 2D 1 1 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod " + "%float %sampled_image %coords123 %depth Lod %float_0", + R"( + Variable{ + Decorations{ + GroupDecoration{0} + BindingDecoration{0} + } + x_10 + none + __sampler_comparison + } + Variable{ + Decorations{ + GroupDecoration{2} + BindingDecoration{1} + } + x_20 + none + __depth_texture_2d_array + })", + R"( + Call[not set]{ + Identifier[not set]{textureSampleCompareLevel} + ( + 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 + MemberAccessor[not set]{ + Identifier[not set]{coords123} + Identifier[not set]{z} + } + } + ScalarConstructor[not set]{0.200000} + ) + })"}, + // 2D, ConstOffset + ImageAccessCase{"%float 2D 1 0 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod %float " + "%sampled_image %coords12 %depth Lod|ConstOffset " + "%float_0 %offsets2d", + R"( + Variable{ + Decorations{ + GroupDecoration{0} + BindingDecoration{0} + } + x_10 + none + __sampler_comparison + } + Variable{ + Decorations{ + GroupDecoration{2} + BindingDecoration{1} + } + x_20 + none + __depth_texture_2d + })", + R"( + Call[not set]{ + Identifier[not set]{textureSampleCompareLevel} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{coords12} + ScalarConstructor[not set]{0.200000} + TypeConstructor[not set]{ + __vec_2__i32 + ScalarConstructor[not set]{3} + ScalarConstructor[not set]{4} + } + ) + })"}, + // 2D array, ConstOffset + ImageAccessCase{"%float 2D 1 1 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod %float " + "%sampled_image %coords123 %depth Lod|ConstOffset " + "%float_0 %offsets2d", + R"( + Variable{ + Decorations{ + GroupDecoration{0} + BindingDecoration{0} + } + x_10 + none + __sampler_comparison + } + Variable{ + Decorations{ + GroupDecoration{2} + BindingDecoration{1} + } + x_20 + none + __depth_texture_2d_array + })", + R"( + Call[not set]{ + Identifier[not set]{textureSampleCompareLevel} + ( + 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 + MemberAccessor[not set]{ + Identifier[not set]{coords123} + Identifier[not set]{z} + } + } + ScalarConstructor[not set]{0.200000} + TypeConstructor[not set]{ + __vec_2__i32 + ScalarConstructor[not set]{3} + ScalarConstructor[not set]{4} + } + ) + })"}, + // Cube + ImageAccessCase{"%float Cube 1 0 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod " + "%float %sampled_image %coords123 %depth Lod %float_0", + R"( + Variable{ + Decorations{ + GroupDecoration{0} + BindingDecoration{0} + } + x_10 + none + __sampler_comparison + } + Variable{ + Decorations{ + GroupDecoration{2} + BindingDecoration{1} + } + x_20 + none + __depth_texture_cube + })", + R"( + Call[not set]{ + Identifier[not set]{textureSampleCompareLevel} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + Identifier[not set]{coords123} + ScalarConstructor[not set]{0.200000} + ) + })"}, + // Cube array + ImageAccessCase{"%float Cube 1 1 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod " + "%float %sampled_image %coords1234 %depth Lod %float_0", + R"( + Variable{ + Decorations{ + GroupDecoration{0} + BindingDecoration{0} + } + x_10 + none + __sampler_comparison + } + Variable{ + Decorations{ + GroupDecoration{2} + BindingDecoration{1} + } + x_20 + none + __depth_texture_cube_array + })", + R"( + Call[not set]{ + Identifier[not set]{textureSampleCompareLevel} + ( + Identifier[not set]{x_20} + Identifier[not set]{x_10} + MemberAccessor[not set]{ + Identifier[not set]{coords1234} + Identifier[not set]{xyz} + } + TypeConstructor[not set]{ + __i32 + MemberAccessor[not set]{ + Identifier[not set]{coords1234} + Identifier[not set]{w} + } + } + ScalarConstructor[not set]{0.200000} + ) + })"})); + INSTANTIATE_TEST_SUITE_P( ImageSampleExplicitLod_UsingLod, SpvParserHandleTest_SampledImageAccessTest, @@ -3156,7 +3402,7 @@ TEST_F(SpvParserHandleTest, ImageWrite_TooFewSrcTexelComponents_1_vs_4) { EXPECT_FALSE(p->BuildAndParseInternalModule()); EXPECT_THAT(p->error(), Eq("texel has too few components for storage texture: 1 provided " - "but 4 required, in: OpImageWrite %53 %3 %2")) + "but 4 required, in: OpImageWrite %54 %3 %2")) << p->error(); } @@ -5365,7 +5611,7 @@ INSTANTIATE_TEST_SUITE_P( {"Identifier[not set]{vf12}\n"}}, {"%float 2D 1 0 0 1 Unknown", "%result = OpImageSampleDrefExplicitLod %float %sampled_image %vf12 " - "%depth Lod %f1", + "%depth Lod %float_0", "", {"Identifier[not set]{vf12}\n"}}, })); @@ -5434,7 +5680,7 @@ INSTANTIATE_TEST_SUITE_P( )"}}, {"%float 2D 1 1 0 1 Unknown", "%result = OpImageSampleDrefExplicitLod %float %sampled_image " - "%vf123 %depth Lod %f1", + "%vf123 %depth Lod %float_0", "", { R"(MemberAccessor[not set]{ @@ -5668,7 +5914,7 @@ INSTANTIATE_TEST_SUITE_P( {"%float 1D 0 0 0 1 Unknown", "%50 = OpCopyObject %float %float_1", "internal error: couldn't find image for " - "%50 = OpCopyObject %18 %44", + "%50 = OpCopyObject %18 %45", {}}, {"%float 1D 0 0 0 1 Unknown", "OpStore %float_var %float_1", @@ -5687,29 +5933,29 @@ INSTANTIATE_TEST_SUITE_P( "%result = OpImageSampleImplicitLod " // bad type for coordinate: not a number "%v4float %sampled_image %float_var", - "bad or unsupported coordinate type for image access: %72 = " - "OpImageSampleImplicitLod %42 %71 %1", + "bad or unsupported coordinate type for image access: %73 = " + "OpImageSampleImplicitLod %42 %72 %1", {}}, {"%float 2D 0 0 0 1 Unknown", // 2D "%result = OpImageSampleImplicitLod " // 1 component, but need 2 "%v4float %sampled_image %f1", "image access required 2 coordinate components, but only 1 provided, " - "in: %72 = OpImageSampleImplicitLod %42 %71 %12", + "in: %73 = OpImageSampleImplicitLod %42 %72 %12", {}}, {"%float 2D 0 1 0 1 Unknown", // 2DArray "%result = OpImageSampleImplicitLod " // 2 component, but need 3 "%v4float %sampled_image %vf12", "image access required 3 coordinate components, but only 2 provided, " - "in: %72 = OpImageSampleImplicitLod %42 %71 %13", + "in: %73 = OpImageSampleImplicitLod %42 %72 %13", {}}, {"%float 3D 0 0 0 1 Unknown", // 3D "%result = OpImageSampleImplicitLod " // 2 components, but need 3 "%v4float %sampled_image %vf12", "image access required 3 coordinate components, but only 2 provided, " - "in: %72 = OpImageSampleImplicitLod %42 %71 %13", + "in: %73 = OpImageSampleImplicitLod %42 %72 %13", {}}, })); @@ -5751,12 +5997,12 @@ INSTANTIATE_TEST_SUITE_P( // ImageSampleDrefExplicitLod {"%uint 2D 0 0 0 1 Unknown", "%result = OpImageSampleDrefExplicitLod %uint %sampled_image %vf12 " - "%f1 Lod %f1", + "%f1 Lod %float_0", "sampled image must have float component type", {}}, {"%int 2D 0 0 0 1 Unknown", "%result = OpImageSampleDrefExplicitLod %int %sampled_image %vf12 " - "%f1 Lod %f1", + "%f1 Lod %float_0", "sampled image must have float component type", {}}})); @@ -5845,6 +6091,43 @@ INSTANTIATE_TEST_SUITE_P( "gradient: ", {}}})); +INSTANTIATE_TEST_SUITE_P( + ImageSampleDrefExplicitLod_CheckForLod0, + // Metal requires comparison sampling with explicit Level-of-detail to use + // Lod 0. The SPIR-V reader requires the operand to be parsed as a constant + // 0 value. SPIR-V validation requires the Lod parameter to be a floating + // point value for non-fetch operations. So only test float values. + SpvParserHandleTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + // float 0.0 works + {"%float 2D 1 0 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod %float %sampled_image %vf1234 " + "%depth Lod %float_0", + "", + {R"(MemberAccessor[not set]{ + Identifier[not set]{vf1234} + Identifier[not set]{xy} +} +)"}}, + // float null works + {"%float 2D 1 0 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod %float %sampled_image %vf1234 " + "%depth Lod %float_0", + "", + {R"(MemberAccessor[not set]{ + Identifier[not set]{vf1234} + Identifier[not set]{xy} +} +)"}}, + // float 1.0 fails. + {"%float 2D 1 0 0 1 Unknown", + "%result = OpImageSampleDrefExplicitLod %float %sampled_image %vf1234 " + "%depth Lod %float_1", + "WGSL comparison sampling without derivatives requires " + "level-of-detail " + "0.0", + {}}})); + TEST_F(SpvParserHandleTest, CombinedImageSampler_IsError) { const auto assembly = Preamble() + R"( OpEntryPoint Fragment %100 "main"