Refactor variable renaming in InlineContentInjector once again

It should now be even better at recognizing conflicts, and avoid renaming
when not necessary.

diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp
index 668f7c5ef2553d399feef9c6c60949990fa54150..0d428149dcc667ca6a7990c5d760a9ca22ea9aee 100644 (file)
--- 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<Statement>::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<Statement>::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<unsigned> set_remap(remap_names, 2);
+               SetForScope<Pass> 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<unsigned> set_remap(remap_names, 1);
-       SetForScope<string> 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<string, VariableDeclaration *>::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<VariableDeclaration> var = new VariableDeclaration;
                var->source = ret.source;

diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h
index 08ee082b99cd9e070dd5003de3cdc936ef21d396..f6604743810ff1e275198ee554badcd53a75ffa2 100644 (file)
--- 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<Statement> r_inlined_statement;
        std::set<Node *> dependencies;
        std::set<std::string> referenced_names;