From 3e262649c1b98462bcaa2c66bc4fb4ee916dc9de Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Tue, 16 Mar 2021 16:10:03 +0200 Subject: [PATCH] Refactor variable renaming in InlineContentInjector once again It should now be even better at recognizing conflicts, and avoid renaming when not necessary. --- source/glsl/optimize.cpp | 79 ++++++++++++++++++++++++++-------------- source/glsl/optimize.h | 10 ++++- 2 files changed, 61 insertions(+), 28 deletions(-) diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp index 668f7c5e..0d428149 100644 --- a/source/glsl/optimize.cpp +++ b/source/glsl/optimize.cpp @@ -66,13 +66,27 @@ void InlineableFunctionLocator::visit(Return &ret) InlineContentInjector::InlineContentInjector(): source_func(0), - remap_names(0) + pass(DEPENDS) { } const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &target_func, Block &tgt_blk, const NodeList::iterator &ins_pt, FunctionDeclaration &src) { - staging_block.parent = &tgt_blk; source_func = &src; + + // Collect all declarations the inlined function depends on. + pass = DEPENDS; + source_func->visit(*this); + + /* Populate referenced_names from the target function so we can rename + variables from the inlined function that would conflict. */ + pass = REFERENCED; + target_func.visit(*this); + + /* Inline and rename passes must be interleaved so used variable names are + known when inlining the return statement. */ + pass = INLINE; + staging_block.parent = &tgt_blk; + staging_block.variables.clear(); remap_prefix = source_func->name; for(NodeList::iterator i=src.body.body.begin(); i!=src.body.body.end(); ++i) @@ -82,14 +96,25 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta if(!r_inlined_statement) r_inlined_statement = (*i)->clone(); - SetForScope set_remap(remap_names, 2); + SetForScope set_pass(pass, RENAME); r_inlined_statement->visit(*this); - staging_block.body.push_back(r_inlined_statement); + + staging_block.body.push_back(0); + staging_block.body.back() = r_inlined_statement; } - SetForScope set_remap(remap_names, 1); - SetForScope set_prefix(remap_prefix, target_func.name); + /* Now collect names from the staging block. Local variables that would + have conflicted with the target function were renamed earlier. */ + pass = REFERENCED; + referenced_names.clear(); staging_block.variables.clear(); + staging_block.visit(*this); + + /* Rename variables in the target function so they don't interfere with + global identifiers used by the source function. */ + pass = RENAME; + staging_block.parent = source_func->body.parent; + remap_prefix = target_func.name; target_func.visit(*this); tgt_blk.body.splice(ins_pt, staging_block.body); @@ -101,40 +126,38 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta void InlineContentInjector::visit(VariableReference &var) { - if(remap_names) + if(pass==RENAME) { map::const_iterator i = staging_block.variables.find(var.name); if(i!=staging_block.variables.end()) var.name = i->second->name; } - else if(var.declaration) + else if(pass==DEPENDS && var.declaration) { - if(!staging_block.variables.count(var.name)) - { - dependencies.insert(var.declaration); - referenced_names.insert(var.name); - } + dependencies.insert(var.declaration); var.declaration->visit(*this); } + else if(pass==REFERENCED) + referenced_names.insert(var.name); } void InlineContentInjector::visit(InterfaceBlockReference &iface) { - if(!remap_names && iface.declaration) + if(pass==DEPENDS && iface.declaration) { dependencies.insert(iface.declaration); - referenced_names.insert(iface.name); iface.declaration->visit(*this); } + else if(pass==REFERENCED) + referenced_names.insert(iface.name); } void InlineContentInjector::visit(FunctionCall &call) { - if(!remap_names && call.declaration) - { + if(pass==DEPENDS && call.declaration) dependencies.insert(call.declaration); + else if(pass==REFERENCED) referenced_names.insert(call.name); - } TraversingVisitor::visit(call); } @@ -142,33 +165,35 @@ void InlineContentInjector::visit(VariableDeclaration &var) { TraversingVisitor::visit(var); - if(remap_names) + if(pass==RENAME) { - if(remap_names==2 || referenced_names.count(var.name)) + staging_block.variables[var.name] = &var; + if(referenced_names.count(var.name)) { string mapped_name = get_unused_variable_name(staging_block, var.name, remap_prefix); - staging_block.variables[var.name] = &var; if(mapped_name!=var.name) + { staging_block.variables[mapped_name] = &var; - var.name = mapped_name; + var.name = mapped_name; + } } } - else if(var.type_declaration) + else if(pass==DEPENDS && var.type_declaration) { dependencies.insert(var.type_declaration); - referenced_names.insert(var.type_declaration->name); var.type_declaration->visit(*this); } + else if(pass==REFERENCED) + referenced_names.insert(var.type); } void InlineContentInjector::visit(Return &ret) { TraversingVisitor::visit(ret); - if(!remap_names && ret.expression) + if(pass==INLINE && ret.expression) { - /* Create a new variable to hold the return value of the inlined - function. */ + // Create a new variable to hold the return value of the inlined function. r_result_name = get_unused_variable_name(staging_block, "_return", source_func->name); RefPtr var = new VariableDeclaration; var->source = ret.source; diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h index 08ee082b..f6604743 100644 --- a/source/glsl/optimize.h +++ b/source/glsl/optimize.h @@ -39,10 +39,18 @@ dependencies of the inlined statements to appear before the target function. */ class InlineContentInjector: private TraversingVisitor { private: + enum Pass + { + DEPENDS, + REFERENCED, + INLINE, + RENAME + }; + FunctionDeclaration *source_func; Block staging_block; std::string remap_prefix; - unsigned remap_names; + Pass pass; RefPtr r_inlined_statement; std::set dependencies; std::set referenced_names; -- 2.43.0