diff --git a/CHANGES b/CHANGES index ffe7b0006..8347371dd 100644 --- a/CHANGES +++ b/CHANGES @@ -16,6 +16,8 @@ v2016.7-dev 2017-01-06 header. #548: Validator: Error when the reserved OpImageSparseSampleProj* opcodes are used. + #611: spvtools::Optimizer was failing to save the module to the output + binary vector when all passes succeded without changes. v2016.6 2016-12-13 - Published the C++ interface for assembling, disassembling, validation, and diff --git a/source/opt/optimizer.cpp b/source/opt/optimizer.cpp index f6dc1cdf4..b1d71973d 100644 --- a/source/opt/optimizer.cpp +++ b/source/opt/optimizer.cpp @@ -75,7 +75,10 @@ bool Optimizer::Run(const uint32_t* original_binary, if (module == nullptr) return false; auto status = impl_->pass_manager.Run(module.get()); - if (status == opt::Pass::Status::SuccessWithChange) { + if (status == opt::Pass::Status::SuccessWithChange || + (status == opt::Pass::Status::SuccessWithoutChange && + (optimized_binary->data() != original_binary || + optimized_binary->size() != original_binary_size))) { optimized_binary->clear(); module->ToBinary(optimized_binary, /* skip_nop = */ true); } diff --git a/test/opt/CMakeLists.txt b/test/opt/CMakeLists.txt index 2f1f482ac..d807d47c5 100644 --- a/test/opt/CMakeLists.txt +++ b/test/opt/CMakeLists.txt @@ -28,6 +28,11 @@ add_spvtools_unittest(TARGET pass_manager LIBS SPIRV-Tools-opt ) +add_spvtools_unittest(TARGET optimizer + SRCS optimizer_test.cpp + LIBS SPIRV-Tools-opt +) + add_spvtools_unittest(TARGET pass_strip_debug_info SRCS strip_debug_info_test.cpp pass_utils.cpp LIBS SPIRV-Tools-opt diff --git a/test/opt/optimizer_test.cpp b/test/opt/optimizer_test.cpp new file mode 100644 index 000000000..ef0e99213 --- /dev/null +++ b/test/opt/optimizer_test.cpp @@ -0,0 +1,109 @@ +// Copyright (c) 2017 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "spirv-tools/libspirv.hpp" +#include "spirv-tools/optimizer.hpp" + +#include "pass_fixture.h" + +namespace { + +using spvtools::CreateNullPass; +using spvtools::CreateStripDebugInfoPass; +using spvtools::Optimizer; +using spvtools::SpirvTools; +using ::testing::Eq; + +TEST(Optimizer, CanRunNullPassWithDistinctInputOutputVectors) { + SpirvTools tools(SPV_ENV_UNIVERSAL_1_0); + std::vector binary_in; + tools.Assemble("OpName %foo \"foo\"\n%foo = OpTypeVoid", &binary_in); + + Optimizer opt(SPV_ENV_UNIVERSAL_1_0); + opt.RegisterPass(CreateNullPass()); + std::vector binary_out; + opt.Run(binary_in.data(), binary_in.size(), &binary_out); + + std::string disassembly; + tools.Disassemble(binary_out.data(), binary_out.size(), &disassembly); + EXPECT_THAT(disassembly, Eq("OpName %foo \"foo\"\n%foo = OpTypeVoid\n")); +} + +TEST(Optimizer, CanRunTransformingPassWithDistinctInputOutputVectors) { + SpirvTools tools(SPV_ENV_UNIVERSAL_1_0); + std::vector binary_in; + tools.Assemble("OpName %foo \"foo\"\n%foo = OpTypeVoid", &binary_in); + + Optimizer opt(SPV_ENV_UNIVERSAL_1_0); + opt.RegisterPass(CreateStripDebugInfoPass()); + std::vector binary_out; + opt.Run(binary_in.data(), binary_in.size(), &binary_out); + + std::string disassembly; + tools.Disassemble(binary_out.data(), binary_out.size(), &disassembly); + EXPECT_THAT(disassembly, Eq("%void = OpTypeVoid\n")); +} + +TEST(Optimizer, CanRunNullPassWithAliasedVectors) { + SpirvTools tools(SPV_ENV_UNIVERSAL_1_0); + std::vector binary; + tools.Assemble("OpName %foo \"foo\"\n%foo = OpTypeVoid", &binary); + + Optimizer opt(SPV_ENV_UNIVERSAL_1_0); + opt.RegisterPass(CreateNullPass()); + opt.Run(binary.data(), binary.size(), &binary); // This is the key. + + std::string disassembly; + tools.Disassemble(binary.data(), binary.size(), &disassembly); + EXPECT_THAT(disassembly, Eq("OpName %foo \"foo\"\n%foo = OpTypeVoid\n")); +} + +TEST(Optimizer, CanRunNullPassWithAliasedVectorDataButDifferentSize) { + SpirvTools tools(SPV_ENV_UNIVERSAL_1_0); + std::vector binary; + tools.Assemble("OpName %foo \"foo\"\n%foo = OpTypeVoid", &binary); + + Optimizer opt(SPV_ENV_UNIVERSAL_1_0); + opt.RegisterPass(CreateNullPass()); + auto orig_size = binary.size(); + // Now change the size. Add a word that will be ignored + // by the optimizer. + binary.push_back(42); + EXPECT_THAT(orig_size + 1, Eq(binary.size())); + opt.Run(binary.data(), orig_size, &binary); // This is the key. + // The binary vector should have been rewritten. + EXPECT_THAT(binary.size(), Eq(orig_size)); + + std::string disassembly; + tools.Disassemble(binary.data(), binary.size(), &disassembly); + EXPECT_THAT(disassembly, Eq("OpName %foo \"foo\"\n%foo = OpTypeVoid\n")); +} + +TEST(Optimizer, CanRunTransformingPassWithAliasedVectors) { + SpirvTools tools(SPV_ENV_UNIVERSAL_1_0); + std::vector binary; + tools.Assemble("OpName %foo \"foo\"\n%foo = OpTypeVoid", &binary); + + Optimizer opt(SPV_ENV_UNIVERSAL_1_0); + opt.RegisterPass(CreateStripDebugInfoPass()); + opt.Run(binary.data(), binary.size(), &binary); // This is the key + + std::string disassembly; + tools.Disassemble(binary.data(), binary.size(), &disassembly); + EXPECT_THAT(disassembly, Eq("%void = OpTypeVoid\n")); +} + +} // namespace