From cd153db8eda941d417dca7201e9009f7a3f3fc82 Mon Sep 17 00:00:00 2001 From: Thomas Roughton Date: Thu, 11 Jul 2019 06:12:19 +1200 Subject: [PATCH] =?UTF-8?q?Add=20=E2=80=94preserve-bindings=20and=20?= =?UTF-8?q?=E2=80=94preserve-spec-constants=20(#2693)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add optimizer options to for preservation of spec constants and variable with binding decorations. They are to be preserved even if they are unused. --- include/spirv-tools/libspirv.h | 9 +++ include/spirv-tools/libspirv.hpp | 12 ++++ source/opt/aggressive_dead_code_elim_pass.cpp | 17 ++++- source/opt/ir_context.h | 25 +++++++- source/opt/optimizer.cpp | 2 + source/spirv_optimizer_options.cpp | 10 +++ source/spirv_optimizer_options.h | 11 +++- test/opt/aggressive_dead_code_elim_test.cpp | 64 +++++++++++++++++++ test/opt/pass_fixture.h | 12 ++++ tools/opt/opt.cpp | 12 ++++ 10 files changed, 170 insertions(+), 4 deletions(-) diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h index acd641ee..e21b0582 100644 --- a/include/spirv-tools/libspirv.h +++ b/include/spirv-tools/libspirv.h @@ -574,6 +574,15 @@ SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetValidatorOptions( SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetMaxIdBound( spv_optimizer_options options, uint32_t val); +// Records whether all bindings within the module should be preserved. +SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetPreserveBindings( + spv_optimizer_options options, bool val); + +// Records whether all specialization constants within the module +// should be preserved. +SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetPreserveSpecConstants( + spv_optimizer_options options, bool val); + // Creates a reducer options object with default options. Returns a valid // options object. The object remains valid until it is passed into // |spvReducerOptionsDestroy|. diff --git a/include/spirv-tools/libspirv.hpp b/include/spirv-tools/libspirv.hpp index a1db4885..2da1152a 100644 --- a/include/spirv-tools/libspirv.hpp +++ b/include/spirv-tools/libspirv.hpp @@ -161,6 +161,18 @@ class OptimizerOptions { spvOptimizerOptionsSetMaxIdBound(options_, new_bound); } + // Records whether all bindings within the module should be preserved. + void set_preserve_bindings(bool preserve_bindings) { + spvOptimizerOptionsSetPreserveBindings(options_, preserve_bindings); + } + + // Records whether all specialization constants within the module + // should be preserved. + void set_preserve_spec_constants(bool preserve_spec_constants) { + spvOptimizerOptionsSetPreserveSpecConstants(options_, + preserve_spec_constants); + } + private: spv_optimizer_options options_; }; diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp index a4bef6c9..bc7ec874 100644 --- a/source/opt/aggressive_dead_code_elim_pass.cpp +++ b/source/opt/aggressive_dead_code_elim_pass.cpp @@ -570,13 +570,28 @@ void AggressiveDCEPass::InitializeModuleScopeLiveInstructions() { AddToWorklist(&entry); } } - // Keep workgroup size. for (auto& anno : get_module()->annotations()) { if (anno.opcode() == SpvOpDecorate) { + // Keep workgroup size. if (anno.GetSingleWordInOperand(1u) == SpvDecorationBuiltIn && anno.GetSingleWordInOperand(2u) == SpvBuiltInWorkgroupSize) { AddToWorklist(&anno); } + + if (context()->preserve_bindings()) { + // Keep all bindings. + if ((anno.GetSingleWordInOperand(1u) == SpvDecorationDescriptorSet) || + (anno.GetSingleWordInOperand(1u) == SpvDecorationBinding)) { + AddToWorklist(&anno); + } + } + + if (context()->preserve_spec_constants()) { + // Keep all specialization constant instructions + if (anno.GetSingleWordInOperand(1u) == SpvDecorationSpecId) { + AddToWorklist(&anno); + } + } } } } diff --git a/source/opt/ir_context.h b/source/opt/ir_context.h index 32d5b179..37c6449c 100644 --- a/source/opt/ir_context.h +++ b/source/opt/ir_context.h @@ -100,7 +100,9 @@ class IRContext { constant_mgr_(nullptr), type_mgr_(nullptr), id_to_name_(nullptr), - max_id_bound_(kDefaultMaxIdBound) { + max_id_bound_(kDefaultMaxIdBound), + preserve_bindings_(false), + preserve_spec_constants_(false) { SetContextMessageConsumer(syntax_context_, consumer_); module_->SetContext(this); } @@ -115,7 +117,9 @@ class IRContext { valid_analyses_(kAnalysisNone), type_mgr_(nullptr), id_to_name_(nullptr), - max_id_bound_(kDefaultMaxIdBound) { + max_id_bound_(kDefaultMaxIdBound), + preserve_bindings_(false), + preserve_spec_constants_(false) { SetContextMessageConsumer(syntax_context_, consumer_); module_->SetContext(this); InitializeCombinators(); @@ -491,6 +495,16 @@ class IRContext { uint32_t max_id_bound() const { return max_id_bound_; } void set_max_id_bound(uint32_t new_bound) { max_id_bound_ = new_bound; } + bool preserve_bindings() const { return preserve_bindings_; } + void set_preserve_bindings(bool should_preserve_bindings) { + preserve_bindings_ = should_preserve_bindings; + } + + bool preserve_spec_constants() const { return preserve_spec_constants_; } + void set_preserve_spec_constants(bool should_preserve_spec_constants) { + preserve_spec_constants_ = should_preserve_spec_constants; + } + // Return id of input variable only decorated with |builtin|, if in module. // Create variable and return its id otherwise. If builtin not currently // supported, return 0. @@ -750,6 +764,13 @@ class IRContext { // The maximum legal value for the id bound. uint32_t max_id_bound_; + + // Whether all bindings within |module_| should be preserved. + bool preserve_bindings_; + + // Whether all specialization constants within |module_| + // should be preserved. + bool preserve_spec_constants_; }; inline IRContext::Analysis operator|(IRContext::Analysis lhs, diff --git a/source/opt/optimizer.cpp b/source/opt/optimizer.cpp index 62d886a6..6206f646 100644 --- a/source/opt/optimizer.cpp +++ b/source/opt/optimizer.cpp @@ -528,6 +528,8 @@ bool Optimizer::Run(const uint32_t* original_binary, if (context == nullptr) return false; context->set_max_id_bound(opt_options->max_id_bound_); + context->set_preserve_bindings(opt_options->preserve_bindings_); + context->set_preserve_spec_constants(opt_options->preserve_spec_constants_); impl_->pass_manager.SetValidatorOptions(&opt_options->val_options_); impl_->pass_manager.SetTargetEnv(impl_->target_env); diff --git a/source/spirv_optimizer_options.cpp b/source/spirv_optimizer_options.cpp index 30db4e2d..e92ffc0f 100644 --- a/source/spirv_optimizer_options.cpp +++ b/source/spirv_optimizer_options.cpp @@ -39,3 +39,13 @@ SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetMaxIdBound( spv_optimizer_options options, uint32_t val) { options->max_id_bound_ = val; } + +SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetPreserveBindings( + spv_optimizer_options options, bool val) { + options->preserve_bindings_ = val; +} + +SPIRV_TOOLS_EXPORT void spvOptimizerOptionsSetPreserveSpecConstants( + spv_optimizer_options options, bool val) { + options->preserve_spec_constants_ = val; +} diff --git a/source/spirv_optimizer_options.h b/source/spirv_optimizer_options.h index 1eb4d3f1..aa76d20b 100644 --- a/source/spirv_optimizer_options.h +++ b/source/spirv_optimizer_options.h @@ -24,7 +24,9 @@ struct spv_optimizer_options_t { spv_optimizer_options_t() : run_validator_(true), val_options_(), - max_id_bound_(kDefaultMaxIdBound) {} + max_id_bound_(kDefaultMaxIdBound), + preserve_bindings_(false), + preserve_spec_constants_(false) {} // When true the validator will be run before optimizations are run. bool run_validator_; @@ -36,5 +38,12 @@ struct spv_optimizer_options_t { // this value must be at least 0x3FFFFF, but implementations can allow for a // higher value. uint32_t max_id_bound_; + + // When true, all binding declarations within the module should be preserved. + bool preserve_bindings_; + + // When true, all specialization constants within the module should be + // preserved. + bool preserve_spec_constants_; }; #endif // SOURCE_SPIRV_OPTIMIZER_OPTIONS_H_ diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp index 4f89487d..f3238015 100644 --- a/test/opt/aggressive_dead_code_elim_test.cpp +++ b/test/opt/aggressive_dead_code_elim_test.cpp @@ -6496,6 +6496,70 @@ OpFunctionEnd SinglePassRunAndMatch(spirv, true); } +TEST_F(AggressiveDCETest, PreserveBindings) { + const std::string spirv = R"( +; CHECK: OpDecorate %unusedSampler DescriptorSet 0 +; CHECK: OpDecorate %unusedSampler Binding 0 +OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 430 +OpName %main "main" +OpName %unusedSampler "unusedSampler" +OpDecorate %unusedSampler DescriptorSet 0 +OpDecorate %unusedSampler Binding 0 +%void = OpTypeVoid +%5 = OpTypeFunction %void +%float = OpTypeFloat 32 +%7 = OpTypeImage %float 2D 0 0 0 1 Unknown +%8 = OpTypeSampledImage %7 +%_ptr_UniformConstant_8 = OpTypePointer UniformConstant %8 +%unusedSampler = OpVariable %_ptr_UniformConstant_8 UniformConstant +%main = OpFunction %void None %5 +%10 = OpLabel +OpReturn +OpFunctionEnd +)"; + + SetTargetEnv(SPV_ENV_UNIVERSAL_1_4); + + OptimizerOptions()->preserve_bindings_ = true; + + SinglePassRunAndMatch(spirv, true); +} + +TEST_F(AggressiveDCETest, PreserveSpecConstants) { + const std::string spirv = R"( +; CHECK: OpName %specConstant "specConstant" +; CHECK: %specConstant = OpSpecConstant %int 0 +OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 430 +OpName %main "main" +OpName %specConstant "specConstant" +OpDecorate %specConstant SpecId 0 +%void = OpTypeVoid +%3 = OpTypeFunction %void +%int = OpTypeInt 32 1 +%specConstant = OpSpecConstant %int 0 +%main = OpFunction %void None %3 +%5 = OpLabel +OpReturn +OpFunctionEnd +)"; + + SetTargetEnv(SPV_ENV_UNIVERSAL_1_4); + + OptimizerOptions()->preserve_spec_constants_ = true; + + SinglePassRunAndMatch(spirv, true); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Check that logical addressing required diff --git a/test/opt/pass_fixture.h b/test/opt/pass_fixture.h index 10e7c53f..b7a07426 100644 --- a/test/opt/pass_fixture.h +++ b/test/opt/pass_fixture.h @@ -27,6 +27,7 @@ #include "source/opt/build_module.h" #include "source/opt/pass_manager.h" #include "source/opt/passes.h" +#include "source/spirv_optimizer_options.h" #include "source/spirv_validator_options.h" #include "source/util/make_unique.h" #include "spirv-tools/libspirv.hpp" @@ -67,6 +68,10 @@ class PassTest : public TestT { return std::make_tuple(std::vector(), Pass::Status::Failure); } + context()->set_preserve_bindings(OptimizerOptions()->preserve_bindings_); + context()->set_preserve_spec_constants( + OptimizerOptions()->preserve_spec_constants_); + const auto status = pass->Run(context()); std::vector binary; @@ -206,6 +211,10 @@ class PassTest : public TestT { std::move(BuildModule(env_, nullptr, original, assemble_options_)); ASSERT_NE(nullptr, context()); + context()->set_preserve_bindings(OptimizerOptions()->preserve_bindings_); + context()->set_preserve_spec_constants( + OptimizerOptions()->preserve_spec_constants_); + manager_->Run(context()); std::vector binary; @@ -232,6 +241,8 @@ class PassTest : public TestT { consumer_ = msg_consumer; } + spv_optimizer_options OptimizerOptions() { return &optimizer_options_; } + spv_validator_options ValidatorOptions() { return &validator_options_; } void SetTargetEnv(spv_target_env env) { env_ = env; } @@ -242,6 +253,7 @@ class PassTest : public TestT { std::unique_ptr manager_; // The pass manager. uint32_t assemble_options_; uint32_t disassemble_options_; + spv_optimizer_options_t optimizer_options_; spv_validator_options_t validator_options_; spv_target_env env_; }; diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index af527e33..d29b8a0a 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -357,6 +357,14 @@ Options (in lexicographical order):)", --merge-blocks followed by all the transformations implied by -O.)"); printf(R"( + --preserve-bindings + Ensure that the optimizer preserves all bindings declared within + the module, even when those bindings are unused.)"); + printf(R"( + --preserve-spec-constants + Ensure that the optimizer preserves all specialization constants declared + within the module, even when those constants are unused.)"); + printf(R"( --print-all Print SPIR-V assembly to standard error output before each pass and after the last pass.)"); @@ -693,6 +701,10 @@ OptStatus ParseFlags(int argc, const char** argv, optimizer_options->set_run_validator(false); } else if (0 == strcmp(cur_arg, "--print-all")) { optimizer->SetPrintAll(&std::cerr); + } else if (0 == strcmp(cur_arg, "--preserve-bindings")) { + optimizer_options->set_preserve_bindings(true); + } else if (0 == strcmp(cur_arg, "--preserve-spec-constants")) { + optimizer_options->set_preserve_spec_constants(true); } else if (0 == strcmp(cur_arg, "--time-report")) { optimizer->SetTimeReport(&std::cerr); } else if (0 == strcmp(cur_arg, "--relax-struct-store")) {