From 03880839dcd2c067061ac5491083159a9fd06611 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Sun, 14 Mar 2021 16:11:07 +0200 Subject: [PATCH] Make variable renaming while inlining more robust It now prevents accidentally hiding global variables too. --- source/glsl/optimize.cpp | 48 +++++++++++++------ source/glsl/optimize.h | 4 +- source/glsl/syntax.cpp | 10 ++-- .../function_inline_global_name_conflict.glsl | 28 +++++++++++ 4 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 tests/glsl/function_inline_global_name_conflict.glsl diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp index c8c9041f..c4118dee 100644 --- a/source/glsl/optimize.cpp +++ b/source/glsl/optimize.cpp @@ -74,6 +74,10 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta { target_block = &tgt_blk; source_func = &src; + remap_prefix = source_func->name; + + vector > inlined; + inlined.reserve(src.body.body.size()); for(NodeList::iterator i=src.body.body.begin(); i!=src.body.body.end(); ++i) { r_inlined_statement = 0; @@ -81,11 +85,18 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta if(!r_inlined_statement) r_inlined_statement = (*i)->clone(); - SetFlag set_remap(remap_names); + SetForScope set_remap(remap_names, 2); r_inlined_statement->visit(*this); - tgt_blk.body.insert(ins_pt, r_inlined_statement); + inlined.push_back(r_inlined_statement); } + SetForScope set_remap(remap_names, 1); + SetForScope set_prefix(remap_prefix, target_func.name); + variable_map.clear(); + target_func.visit(*this); + + tgt_blk.body.insert(ins_pt, inlined.begin(), inlined.end()); + NodeReorderer().apply(stage, target_func, dependencies); return r_result_name; @@ -102,7 +113,11 @@ void InlineContentInjector::visit(VariableReference &var) else if(var.declaration) { SetFlag set_deps(deps_only); - dependencies.insert(var.declaration); + if(!variable_map.count(var.name)) + { + dependencies.insert(var.declaration); + referenced_names.insert(var.name); + } var.declaration->visit(*this); } } @@ -113,6 +128,7 @@ void InlineContentInjector::visit(InterfaceBlockReference &iface) { SetFlag set_deps(deps_only); dependencies.insert(iface.declaration); + referenced_names.insert(iface.name); iface.declaration->visit(*this); } } @@ -120,7 +136,10 @@ void InlineContentInjector::visit(InterfaceBlockReference &iface) void InlineContentInjector::visit(FunctionCall &call) { if(!remap_names && call.declaration) + { dependencies.insert(call.declaration); + referenced_names.insert(call.name); + } TraversingVisitor::visit(call); } @@ -128,28 +147,29 @@ void InlineContentInjector::visit(VariableDeclaration &var) { TraversingVisitor::visit(var); - if(var.type_declaration) + if(remap_names) + { + if(remap_names==2 || referenced_names.count(var.name)) + { + string mapped_name = get_unused_variable_name(*target_block, var.name, remap_prefix); + variable_map[var.name] = &var; + var.name = mapped_name; + } + } + else if(var.type_declaration) { SetFlag set_deps(deps_only); dependencies.insert(var.type_declaration); + referenced_names.insert(var.type_declaration->name); var.type_declaration->visit(*this); } - - if(!remap_names && !deps_only) - { - RefPtr inlined_var = var.clone(); - inlined_var->name = get_unused_variable_name(*target_block, var.name, source_func->name); - r_inlined_statement = inlined_var; - - variable_map[var.name] = inlined_var.get(); - } } void InlineContentInjector::visit(Return &ret) { TraversingVisitor::visit(ret); - if(ret.expression) + if(!remap_names && ret.expression) { /* Create a new variable to hold the return value of the inlined function. */ diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h index ab954b58..8a888e15 100644 --- a/source/glsl/optimize.h +++ b/source/glsl/optimize.h @@ -43,10 +43,12 @@ private: FunctionDeclaration *source_func; Block *target_block; std::map variable_map; - bool remap_names; + std::string remap_prefix; + unsigned remap_names; bool deps_only; RefPtr r_inlined_statement; std::set dependencies; + std::set referenced_names; std::string r_result_name; public: diff --git a/source/glsl/syntax.cpp b/source/glsl/syntax.cpp index cb50fee1..c94e2e27 100644 --- a/source/glsl/syntax.cpp +++ b/source/glsl/syntax.cpp @@ -472,8 +472,14 @@ string get_unused_variable_name(const Block &block, const string &base, const st bool prefixed = false; unsigned number = 1; unsigned size_without_number = name.size(); - while(block.variables.count(name)) + while(1) { + bool unused = true; + for(const Block *b=█ (unused && b); b=b->parent) + unused = !b->variables.count(name); + if(unused) + return name; + if(!prefixed && !prefix_hint.empty()) { if(name.front()!='_') @@ -491,8 +497,6 @@ string get_unused_variable_name(const Block &block, const string &base, const st ++number; } } - - return name; } } // namespace SL diff --git a/tests/glsl/function_inline_global_name_conflict.glsl b/tests/glsl/function_inline_global_name_conflict.glsl new file mode 100644 index 00000000..8ebbfb46 --- /dev/null +++ b/tests/glsl/function_inline_global_name_conflict.glsl @@ -0,0 +1,28 @@ +#pragma MSP stage(vertex) +layout(location=0) in vec4 position; +layout(location=1) in float scale; +layout(location=2) in float size; +float func() +{ + float scale = size*2.0; + return scale*scale; +} +void main() +{ + float size = scale+1.0; + float s = func()+1.0; + gl_Position = position*size*size*s*s; +} + +/* Expected output: vertex +layout(location=0) in vec4 position; +layout(location=1) in float scale; +layout(location=2) in float size; +void main() +{ + float _main_size = scale+1.0; + float _func_scale = size*2.0; + float s = _func_scale*_func_scale+1.0; + gl_Position = position*_main_size*_main_size*s*s; +} +*/ -- 2.43.0