From b97e9b0499f299e140271498302218f7ef6920ac Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Thu, 1 Aug 2019 09:51:44 +0200 Subject: [PATCH] Fix severe performance issue with invariant expression invalidation. We were going down a tree of expressions multiple times and this caused an exponential explosion in time, which was not caught until recently. Fix this by blocking any traversal going through an ID more than one time. This fix overall improves performance by almost an order of magnitude on a particular test shader rather than slowing it down by ~75x. --- main.cpp | 3 ++- spirv_cross.hpp | 1 + spirv_glsl.cpp | 13 ++++++++----- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/main.cpp b/main.cpp index 43e8cf79..3d97247e 100644 --- a/main.cpp +++ b/main.cpp @@ -1063,7 +1063,8 @@ static int main_inner(int argc, char *argv[]) cbs.add("--metal", [&args](CLIParser &) { args.msl = true; }); // Legacy compatibility cbs.add("--glsl-emit-push-constant-as-ubo", [&args](CLIParser &) { args.glsl_emit_push_constant_as_ubo = true; }); cbs.add("--glsl-emit-ubo-as-plain-uniforms", [&args](CLIParser &) { args.glsl_emit_ubo_as_plain_uniforms = true; }); - cbs.add("--vulkan-glsl-disable-ext-samplerless-texture-functions", [&args](CLIParser &) { args.vulkan_glsl_disable_ext_samplerless_texture_functions = true; }); + cbs.add("--vulkan-glsl-disable-ext-samplerless-texture-functions", + [&args](CLIParser &) { args.vulkan_glsl_disable_ext_samplerless_texture_functions = true; }); cbs.add("--msl", [&args](CLIParser &) { args.msl = true; }); cbs.add("--hlsl", [&args](CLIParser &) { args.hlsl = true; }); cbs.add("--hlsl-enable-compat", [&args](CLIParser &) { args.hlsl_compat = true; }); diff --git a/spirv_cross.hpp b/spirv_cross.hpp index 7797bf25..ca75dc66 100644 --- a/spirv_cross.hpp +++ b/spirv_cross.hpp @@ -816,6 +816,7 @@ protected: std::unordered_set forwarded_temporaries; std::unordered_set suppressed_usage_tracking; std::unordered_set hoisted_temporaries; + std::unordered_set forced_invariant_temporaries; Bitset active_input_builtins; Bitset active_output_builtins; diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index f83f7c6a..bf33d83b 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -4981,7 +4981,8 @@ std::string CompilerGLSL::convert_separate_image_to_expression(uint32_t id) // Don't need to consider Shadow state since the dummy sampler is always non-shadow. auto sampled_type = type; sampled_type.basetype = SPIRType::SampledImage; - return join(type_to_glsl(sampled_type), "(", to_expression(id), ", ", to_expression(dummy_sampler_id), ")"); + return join(type_to_glsl(sampled_type), "(", to_expression(id), ", ", + to_expression(dummy_sampler_id), ")"); } else { @@ -7550,14 +7551,16 @@ void CompilerGLSL::disallow_forwarding_in_expression_chain(const SPIRExpression // Allow trivially forwarded expressions like OpLoad or trivial shuffles, // these will be marked as having suppressed usage tracking. // Our only concern is to make sure arithmetic operations are done in similar ways. - if (expression_is_forwarded(expr.self) && !expression_suppresses_usage_tracking(expr.self)) + if (expression_is_forwarded(expr.self) && !expression_suppresses_usage_tracking(expr.self) && + forced_invariant_temporaries.count(expr.self) == 0) { forced_temporaries.insert(expr.self); + forced_invariant_temporaries.insert(expr.self); force_recompile(); - } - for (auto &dependent : expr.expression_dependencies) - disallow_forwarding_in_expression_chain(get(dependent)); + for (auto &dependent : expr.expression_dependencies) + disallow_forwarding_in_expression_chain(get(dependent)); + } } void CompilerGLSL::handle_store_to_invariant_variable(uint32_t store_id, uint32_t value_id)