From b2c2e6483b7a78b995279a57b103a7029209a1bc Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Sat, 25 Mar 2017 16:25:30 +0100 Subject: [PATCH] Analyze parameter preservation for functions. This is kinda tricky, because if we only conditionally write to a function parameter variable it is implicitly preserved in SPIR-V, so we must force an in qualifier on the parameter to get the same behavior in GLSL. --- .../shaders/comp/cfg-preserve-parameter.comp | 74 +++++++++++++++++++ shaders/comp/cfg-preserve-parameter.comp | 54 ++++++++++++++ spirv_cfg.cpp | 1 + spirv_cfg.hpp | 3 +- spirv_common.hpp | 3 + spirv_cross.cpp | 69 +++++++++++++++++ spirv_cross.hpp | 3 + 7 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 reference/shaders/comp/cfg-preserve-parameter.comp create mode 100644 shaders/comp/cfg-preserve-parameter.comp diff --git a/reference/shaders/comp/cfg-preserve-parameter.comp b/reference/shaders/comp/cfg-preserve-parameter.comp new file mode 100644 index 0000000..72bde44 --- /dev/null +++ b/reference/shaders/comp/cfg-preserve-parameter.comp @@ -0,0 +1,74 @@ +#version 310 es +layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; + +void out_test_0(int cond, out int i) +{ + if (cond == 0) + { + i = 40; + } + else + { + i = 60; + } +} + +void out_test_1(int cond, out int i) +{ + switch (cond) + { + case 40: + { + i = 40; + break; + } + default: + { + i = 70; + break; + } + } +} + +void inout_test_0(int cond, inout int i) +{ + if (cond == 0) + { + i = 40; + } +} + +void inout_test_1(int cond, inout int i) +{ + switch (cond) + { + case 40: + { + i = 40; + break; + } + } +} + +void main() +{ + int cond = 40; + int i = 50; + int param = cond; + int param_1 = i; + out_test_0(param, param_1); + i = param_1; + int param_2 = cond; + int param_3 = i; + out_test_1(param_2, param_3); + i = param_3; + int param_4 = cond; + int param_5 = i; + inout_test_0(param_4, param_5); + i = param_5; + int param_6 = cond; + int param_7 = i; + inout_test_1(param_6, param_7); + i = param_7; +} + diff --git a/shaders/comp/cfg-preserve-parameter.comp b/shaders/comp/cfg-preserve-parameter.comp new file mode 100644 index 0000000..9ef9092 --- /dev/null +++ b/shaders/comp/cfg-preserve-parameter.comp @@ -0,0 +1,54 @@ +#version 310 es + +// We write in all paths (and no reads), so should just be out. +void out_test_0(int cond, inout int i) +{ + if (cond == 0) + i = 40; + else + i = 60; +} + +// We write in all paths (and no reads), so should just be out. +void out_test_1(int cond, inout int i) +{ + switch (cond) + { + case 40: + i = 40; + break; + + default: + i = 70; + break; + } +} + +// We don't write in all paths, so should be inout. +void inout_test_0(int cond, inout int i) +{ + if (cond == 0) + i = 40; +} + +void inout_test_1(int cond, inout int i) +{ + switch (cond) + { + case 40: + i = 40; + break; + } +} + + +void main() +{ + int cond = 40; + int i = 50; + + out_test_0(cond, i); + out_test_1(cond, i); + inout_test_0(cond, i); + inout_test_1(cond, i); +} diff --git a/spirv_cfg.cpp b/spirv_cfg.cpp index b7abf47..32549ed 100644 --- a/spirv_cfg.cpp +++ b/spirv_cfg.cpp @@ -15,6 +15,7 @@ */ #include "spirv_cfg.hpp" +#include "spirv_cross.hpp" #include #include diff --git a/spirv_cfg.hpp b/spirv_cfg.hpp index 882f7d3..8b6a1a7 100644 --- a/spirv_cfg.hpp +++ b/spirv_cfg.hpp @@ -17,11 +17,12 @@ #ifndef SPIRV_CROSS_CFG_HPP #define SPIRV_CROSS_CFG_HPP -#include "spirv_cross.hpp" #include +#include "spirv_common.hpp" namespace spirv_cross { +class Compiler; class CFG { public: diff --git a/spirv_common.hpp b/spirv_common.hpp index d7d037f..a92ff60 100644 --- a/spirv_common.hpp +++ b/spirv_common.hpp @@ -22,6 +22,9 @@ #include #include #include +#include +#include +#include "spirv.hpp" namespace spirv_cross { diff --git a/spirv_cross.cpp b/spirv_cross.cpp index 8119c34..d7d9827 100644 --- a/spirv_cross.cpp +++ b/spirv_cross.cpp @@ -2802,6 +2802,72 @@ const SPIRConstant &Compiler::get_constant(uint32_t id) const return get(id); } +static bool exists_unaccessed_path_to_return(const CFG &cfg, uint32_t block, + const unordered_set &blocks) +{ + // This block accesses the variable. + if (blocks.find(block) != end(blocks)) + return false; + + // We are at the end of the CFG. + if (cfg.get_succeeding_edges(block).empty()) + return true; + + // If any of our successors have a path to the end, there exists a path from block. + for (auto &succ : cfg.get_succeeding_edges(block)) + if (exists_unaccessed_path_to_return(cfg, succ, blocks)) + return true; + + return false; +} + +void Compiler::analyze_parameter_preservation(SPIRFunction &entry, const CFG &cfg, + const unordered_map> &variable_to_blocks) +{ + for (auto &arg : entry.arguments) + { + // Non-pointers are always inputs. + auto &type = get(arg.type); + if (!type.pointer) + continue; + + // Opaque argument types are always in + bool potential_preserve; + switch (type.basetype) + { + case SPIRType::Sampler: + case SPIRType::Image: + case SPIRType::SampledImage: + case SPIRType::AtomicCounter: + potential_preserve = false; + break; + + default: + potential_preserve = true; + break; + } + + if (!potential_preserve) + continue; + + auto itr = variable_to_blocks.find(arg.id); + if (itr == end(variable_to_blocks)) + { + // Variable is never accessed. + continue; + } + + // If there is a path through the CFG where no block writes to the variable, the variable will be in an undefined state + // when the function returns. We therefore need to implicitly preserve the variable in case there are writers in the function. + // Major case here is if a function is + // void foo(int &var) { if (cond) var = 10; } + // Using read/write counts, we will think it's just an out variable, but it really needs to be inout, + // because if we don't write anything whatever we put into the function must return back to the caller. + if (exists_unaccessed_path_to_return(cfg, entry.entry_block, itr->second)) + arg.read_count++; + } +} + void Compiler::analyze_variable_scope(SPIRFunction &entry) { struct AccessHandler : OpcodeHandler @@ -2971,6 +3037,9 @@ void Compiler::analyze_variable_scope(SPIRFunction &entry) // Compute the control flow graph for this function. CFG cfg(*this, entry); + // Analyze if there are parameters which need to be implicitly preserved with an "in" qualifier. + analyze_parameter_preservation(entry, cfg, handler.accessed_variables_to_block); + unordered_map potential_loop_variables; // For each variable which is statically accessed. diff --git a/spirv_cross.hpp b/spirv_cross.hpp index 26d22dd..e6c8be5 100644 --- a/spirv_cross.hpp +++ b/spirv_cross.hpp @@ -28,6 +28,7 @@ #include #include "spirv_common.hpp" +#include "spirv_cfg.hpp" namespace spirv_cross { @@ -614,6 +615,8 @@ protected: uint64_t active_output_builtins = 0; // Traverses all reachable opcodes and sets active_builtins to a bitmask of all builtin variables which are accessed in the shader. void update_active_builtins(); + + void analyze_parameter_preservation(SPIRFunction &entry, const CFG &cfg, const std::unordered_map> &variable_to_blocks); }; }