diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 97907ab0e7..92f83e5f24 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -500,6 +500,22 @@ bool IsSampledImageAccess(SpvOp opcode) { return false; } +// @param opcode a SPIR-V opcode +// @returns true if the given instruction is an image sampling operation. +bool IsImageSampling(SpvOp opcode) { + switch (opcode) { + case SpvOpImageSampleImplicitLod: + case SpvOpImageSampleExplicitLod: + case SpvOpImageSampleDrefImplicitLod: + case SpvOpImageSampleDrefExplicitLod: + return true; + default: + // WGSL doesn't have *Proj* texturing. + break; + } + return false; +} + // @param opcode a SPIR-V opcode // @returns true if the given instruction is an image access instruction // whose first input operand is an OpImage value. @@ -3989,7 +4005,8 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { params.push_back(create( Source{}, ast_module_.RegisterSymbol(name), name)); - if (IsSampledImageAccess(inst.opcode())) { + const auto opcode = inst.opcode(); + if (IsSampledImageAccess(opcode)) { // Form the sampler operand. const auto* sampler = parser_impl_.GetMemoryObjectDeclarationForHandle( image_or_sampled_image_operand_id, false); @@ -4029,7 +4046,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { std::string builtin_name; bool use_load_suffix = true; - switch (inst.opcode()) { + switch (opcode) { case SpvOpImageSampleImplicitLod: case SpvOpImageSampleExplicitLod: builtin_name = "textureSample"; @@ -4121,12 +4138,28 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { } if (arg_index < num_args && (image_operands_mask & SpvImageOperandsConstOffsetMask)) { + if (!IsImageSampling(opcode)) { + return Fail() << "ConstOffset is only permitted for sampling operations: " + << inst.PrettyPrint(); + } + switch (texture_type->dim()) { + case ast::type::TextureDimension::k2d: + case ast::type::TextureDimension::k2dArray: + case ast::type::TextureDimension::k3d: + break; + default: + return Fail() << "ConstOffset is only permitted for 2D, 2D Arrayed, " + "and 3D textures: " + << inst.PrettyPrint(); + } + params.push_back(ToSignedIfUnsigned(MakeOperand(inst, arg_index)).expr); image_operands_mask ^= SpvImageOperandsConstOffsetMask; arg_index++; } if (arg_index < num_args && (image_operands_mask & SpvImageOperandsSampleMask)) { + // TODO(dneto): only permitted with ImageFetch params.push_back(ToI32(MakeOperand(inst, arg_index)).expr); image_operands_mask ^= SpvImageOperandsSampleMask; arg_index++; @@ -4184,7 +4217,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { value = create(Source{}, result_type, call_expr); } if (!expected_component_type->Is() && - IsSampledImageAccess(inst.opcode())) { + IsSampledImageAccess(opcode)) { // WGSL permits sampled image access only on float textures. // Reject this case in the SPIR-V reader, at least until SPIR-V validation // catches up with this rule and can reject it earlier in the workflow. diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc index 1cf0f1fb28..d4ad95a417 100644 --- a/src/reader/spirv/parser_impl_handle_test.cc +++ b/src/reader/spirv/parser_impl_handle_test.cc @@ -30,6 +30,7 @@ namespace { using ::testing::Eq; using ::testing::HasSubstr; using ::testing::Not; +using ::testing::StartsWith; std::string Preamble() { return R"( @@ -2617,28 +2618,6 @@ INSTANTIATE_TEST_SUITE_P( Identifier[not set]{vi12} Identifier[not set]{vf1234} ) - })"}, - // OpImageWrite with ConstOffset - {"%float 2D 0 0 0 2 Rgba32f", - "OpImageWrite %im %vi12 %vf1234 ConstOffset %offsets2d", - R"( - Variable{ - Decorations{ - SetDecoration{2} - BindingDecoration{1} - } - x_20 - uniform_constant - __storage_texture_write_only_2d_rgba32float - })", - R"(Call[not set]{ - Identifier[not set]{textureStore} - ( - Identifier[not set]{x_20} - Identifier[not set]{vi12} - Identifier[not set]{vf1234} - Identifier[not set]{offsets2d} - ) })"}})); INSTANTIATE_TEST_SUITE_P( @@ -3085,35 +3064,6 @@ INSTANTIATE_TEST_SUITE_P( } } } - })"}, - // OpImageRead with ConstOffset - {"%float 2D 0 0 0 2 Rgba32f", - "%99 = OpImageRead %v4float %im %vi12 ConstOffset %offsets2d", - R"(Variable{ - Decorations{ - SetDecoration{2} - BindingDecoration{1} - } - x_20 - uniform_constant - __storage_texture_read_only_2d_rgba32float - })", - R"(VariableDeclStatement{ - VariableConst{ - x_99 - none - __vec_4__f32 - { - Call[not set]{ - Identifier[not set]{textureLoad} - ( - Identifier[not set]{x_20} - Identifier[not set]{vi12} - Identifier[not set]{offsets2d} - ) - } - } - } })"}})); INSTANTIATE_TEST_SUITE_P( @@ -3146,36 +3096,6 @@ INSTANTIATE_TEST_SUITE_P( } } } - })"}, - // OpImageFetch with ConstOffset - // TODO(dneto): Seems this is not valid in WGSL. - {"%float 2D 0 0 0 1 Unknown", - "%99 = OpImageFetch %v4float %im %vi12 ConstOffset %offsets2d", - R"(Variable{ - Decorations{ - SetDecoration{2} - BindingDecoration{1} - } - x_20 - uniform_constant - __sampled_texture_2d__f32 - })", - R"(VariableDeclStatement{ - VariableConst{ - x_99 - none - __vec_4__f32 - { - Call[not set]{ - Identifier[not set]{textureLoad} - ( - Identifier[not set]{x_20} - Identifier[not set]{vi12} - Identifier[not set]{offsets2d} - ) - } - } - } })"}, // OpImageFetch with explicit level {"%float 2D 0 0 0 1 Unknown", @@ -3760,7 +3680,7 @@ TEST_P(SpvParserTest_ImageCoordsTest, MakeCoordinateOperandsForImageAccess) { )"; auto p = parser(test::Assemble(assembly)); if (!p->BuildAndParseInternalModule()) { - EXPECT_THAT(p->error(), Eq(GetParam().expected_error)) << assembly; + EXPECT_THAT(p->error(), StartsWith(GetParam().expected_error)) << assembly; } else { EXPECT_TRUE(p->error().empty()) << p->error(); FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100)); @@ -4456,6 +4376,59 @@ INSTANTIATE_TEST_SUITE_P( "sampled image must have float component type", {}}})); +INSTANTIATE_TEST_SUITE_P( + ConstOffset_BadInstruction_Errors, + SpvParserTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + // ImageFetch + {"%uint 2D 0 0 0 1 Unknown", + "%result = OpImageFetch %v4uint %sampled_image %vf12 ConstOffset " + "%the_vu12", + "ConstOffset is only permitted for sampling operations: ", + {}}, + // ImageRead + {"%uint 2D 0 0 0 2 Rgba32ui", + "%result = OpImageRead %v4uint %im %vu12 ConstOffset %the_vu12", + "ConstOffset is only permitted for sampling operations: ", + {}}, + // ImageWrite + {"%uint 2D 0 0 0 2 Rgba32ui", + "OpImageWrite %im %vu12 %vu1234 ConstOffset %the_vu12", + "ConstOffset is only permitted for sampling operations: ", + {}} + // TODO(dneto): Gather + // TODO(dneto): DrefGather + })); + +INSTANTIATE_TEST_SUITE_P( + ConstOffset_BadDim_Errors, + SpvParserTest_ImageCoordsTest, + ::testing::ValuesIn(std::vector{ + // 1D + {"%uint 1D 0 0 0 1 Unknown", + "%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 " + "ConstOffset %the_vu12", + "ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ", + {}}, + // 1D Array + {"%uint 1D 0 1 0 1 Unknown", + "%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 " + "ConstOffset %the_vu12", + "ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ", + {}}, + // Cube + {"%uint Cube 0 0 0 1 Unknown", + "%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 " + "ConstOffset %the_vu12", + "ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ", + {}}, + // Cube Array + {"%uint Cube 0 1 0 1 Unknown", + "%result = OpImageSampleImplicitLod %v4float %sampled_image %vf1234 " + "ConstOffset %the_vu12", + "ConstOffset is only permitted for 2D, 2D Arrayed, and 3D textures: ", + {}}})); + } // namespace } // namespace spirv } // namespace reader