diff --git a/third_party/spirv-tools/source/opt/split_combined_image_sampler_pass.cpp b/third_party/spirv-tools/source/opt/split_combined_image_sampler_pass.cpp index e454e3b96b..0af4dba6cb 100644 --- a/third_party/spirv-tools/source/opt/split_combined_image_sampler_pass.cpp +++ b/third_party/spirv-tools/source/opt/split_combined_image_sampler_pass.cpp @@ -358,6 +358,13 @@ spv_result_t SplitCombinedImageSamplerPass::RemapUses( use.image_part->result_id()); auto* sampler = builder.AddLoad(PointeeTypeId(use.sampler_part), use.sampler_part->result_id()); + + // Move decorations, such as RelaxedPrecision. + auto* deco_mgr = context()->get_decoration_mgr(); + deco_mgr->CloneDecorations(load->result_id(), image->result_id()); + deco_mgr->CloneDecorations(load->result_id(), sampler->result_id()); + deco_mgr->RemoveDecorationsFrom(load->result_id()); + // Create a sampled image from the loads of the two parts. auto* sampled_image = builder.AddSampledImage( load->type_id(), image->result_id(), sampler->result_id()); diff --git a/third_party/spirv-tools/test/opt/pass_fixture.h b/third_party/spirv-tools/test/opt/pass_fixture.h index 32e529608d..2584e10780 100644 --- a/third_party/spirv-tools/test/opt/pass_fixture.h +++ b/third_party/spirv-tools/test/opt/pass_fixture.h @@ -122,23 +122,24 @@ class PassTest : public TestT { auto status = Pass::Status::SuccessWithoutChange; std::tie(optimized_bin, status) = SinglePassRunToBinary( assembly, skip_nop, std::forward(args)...); - if (do_validation) { - spv_context spvContext = spvContextCreate(env_); - spv_diagnostic diagnostic = nullptr; - spv_const_binary_t binary = {optimized_bin.data(), optimized_bin.size()}; - spv_result_t error = spvValidateWithOptions( - spvContext, ValidatorOptions(), &binary, &diagnostic); - EXPECT_EQ(error, 0); - if (error != 0) spvDiagnosticPrint(diagnostic); - spvDiagnosticDestroy(diagnostic); - spvContextDestroy(spvContext); - } std::string optimized_asm; SpirvTools tools(env_); EXPECT_TRUE( tools.Disassemble(optimized_bin, &optimized_asm, disassemble_options_)) << "Disassembling failed for shader:\n" << assembly << std::endl; + if (do_validation) { + spv_context spvContext = spvContextCreate(env_); + spv_diagnostic diagnostic = nullptr; + spv_const_binary_t binary = {optimized_bin.data(), optimized_bin.size()}; + spv_result_t error = spvValidateWithOptions( + spvContext, ValidatorOptions(), &binary, &diagnostic); + EXPECT_EQ(error, 0) << "validation failed for optimized asm:\n" + << optimized_asm; + if (error != 0) spvDiagnosticPrint(diagnostic); + spvDiagnosticDestroy(diagnostic); + spvContextDestroy(spvContext); + } return std::make_tuple(optimized_asm, status); } diff --git a/third_party/spirv-tools/test/opt/split_combined_image_sampler_pass_test.cpp b/third_party/spirv-tools/test/opt/split_combined_image_sampler_pass_test.cpp index 2ffffa5ad6..d6ad6bb5f2 100644 --- a/third_party/spirv-tools/test/opt/split_combined_image_sampler_pass_test.cpp +++ b/third_party/spirv-tools/test/opt/split_combined_image_sampler_pass_test.cpp @@ -386,8 +386,8 @@ TEST_P(SplitCombinedImageSamplerPassTypeCaseTest, Combined_RemapLoad) { } TEST_P(SplitCombinedImageSamplerPassTypeCaseTest, - Combined_RemapLoad_RelaxedPrecisionCopied) { - // All decorations are copied. In this case, RelaxedPrecision + Combined_RemapLoad_RelaxedPrecisionOnVarCopied) { + // All decorations on the variable are copied. In this case, RelaxedPrecision const std::string kTest = Preamble() + R"( OpName %combined "combined" @@ -412,6 +412,7 @@ TEST_P(SplitCombinedImageSamplerPassTypeCaseTest, ; The combined image variable is replaced by an image variable and a sampler variable. ; CHECK-NOT: %100 = OpVariable + ; CHECK-NOT: OpVariable _ptr_UniformConstant_11 ; CHECK-DAG: %[[sampler_var]] = OpVariable %[[sampler_ptr_ty]] UniformConstant ; CHECK-DAG: %[[image_var]] = OpVariable %[[image_ptr_ty]] UniformConstant ; CHECK: = OpFunction @@ -445,6 +446,70 @@ TEST_P(SplitCombinedImageSamplerPassTypeCaseTest, EXPECT_EQ(status, Pass::Status::SuccessWithChange) << disasm; } +TEST_P(SplitCombinedImageSamplerPassTypeCaseTest, + Combined_RemapLoad_RelaxedPrecisionOnLoadCopied) { + // Copy decorations form an OpLoad that is replaced. + const std::string kTest = Preamble() + + R"( + OpName %combined "combined" + OpDecorate %100 DescriptorSet 0 + OpDecorate %100 Binding 0 + OpDecorate %combined RelaxedPrecision + + ; CHECK: OpName + ; CHECK-NOT: OpDecorate %100 + ; CHECK: OpDecorate %[[image_var:\d+]] DescriptorSet 0 + ; CHECK: OpDecorate %[[sampler_var:\d+]] DescriptorSet 0 + ; CHECK: OpDecorate %[[image_var]] Binding 0 + ; CHECK: OpDecorate %[[sampler_var]] Binding 0 + + ; This is what we are checking in this test. + ; CHECK: OpDecorate %[[im:\d+]] RelaxedPrecision + ; CHECK: OpDecorate %[[s:\d+]] RelaxedPrecision + + ; CHECK: %10 = OpTypeImage % + ; CHECK: %[[image_ptr_ty:\w+]] = OpTypePointer UniformConstant %10 + ; CHECK: %[[sampler_ty:\d+]] = OpTypeSampler + ; CHECK: %[[sampler_ptr_ty:\w+]] = OpTypePointer UniformConstant %[[sampler_ty]] + + ; The combined image variable is replaced by an image variable and a sampler variable. + + ; CHECK-NOT: %100 = OpVariable + ; CHECK-NOT: OpVariable _ptr_UniformConstant_11 + ; CHECK-DAG: %[[sampler_var]] = OpVariable %[[sampler_ptr_ty]] UniformConstant + ; CHECK-DAG: %[[image_var]] = OpVariable %[[image_ptr_ty]] UniformConstant + ; CHECK: = OpFunction + + ; The load of the combined image+sampler is replaced by a two loads, then + ; a combination operation. The new loads get the same decorations that the + ; original load had. + ; CHECK: %[[im]] = OpLoad %10 %[[image_var]] + ; CHECK: %[[s]] = OpLoad %[[sampler_ty]] %[[sampler_var]] + ; CHECK: %combined = OpSampledImage %11 %[[im]] %[[s]] + + %bool = OpTypeBool ; location marker +)" + BasicTypes() + + " %10 = " + GetParam().image_type_decl + R"( + %11 = OpTypeSampledImage %10 +%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11 + + %100 = OpVariable %_ptr_UniformConstant_11 UniformConstant + %main = OpFunction %void None %voidfn + %main_0 = OpLabel + %combined = OpLoad %11 %100 + + ; Uses of the combined image sampler are preserved. + ; CHECK: OpCopyObject %11 %combined + + %7 = OpCopyObject %11 %combined + OpReturn + OpFunctionEnd +)"; + auto [disasm, status] = SinglePassRunAndMatch( + kTest, /* do_validation= */ true); + EXPECT_EQ(status, Pass::Status::SuccessWithChange) << disasm; +} + TEST_P(SplitCombinedImageSamplerPassTypeCaseTest, Combined_DeletesCopyObjectOfPtr) { // OpCopyObject is deleted, and its uses updated.