]> git.tdb.fi Git - libs/gl.git/blobdiff - source/glsl/optimize.cpp
Transform interface block contents into structs
[libs/gl.git] / source / glsl / optimize.cpp
index fcb1075ecbadc6a1d412142dc8191a83587ff917..c1431f2b6e9d9e33ed522cd853bcd326bbb3414a 100644 (file)
@@ -1,4 +1,5 @@
 #include <msp/core/raii.h>
+#include <msp/strings/format.h>
 #include "optimize.h"
 
 using namespace std;
@@ -8,20 +9,23 @@ namespace GL {
 namespace SL {
 
 InlineableFunctionLocator::InlineableFunctionLocator():
-       in_function(0)
+       current_function(0),
+       return_count(0)
 { }
 
 void InlineableFunctionLocator::visit(FunctionCall &call)
 {
        FunctionDeclaration *def = call.declaration;
-       if(def && def->definition!=def)
+       if(def)
                def = def->definition;
 
        if(def)
        {
                unsigned &count = refcounts[def];
                ++count;
-               if(count>1 || def==in_function)
+               /* Don't inline functions which are called more than once or are called
+               recursively. */
+               if(count>1 || def==current_function)
                        inlineable.erase(def);
        }
 
@@ -31,62 +35,201 @@ void InlineableFunctionLocator::visit(FunctionCall &call)
 void InlineableFunctionLocator::visit(FunctionDeclaration &func)
 {
        unsigned &count = refcounts[func.definition];
-       if(!count && func.parameters.empty())
+       if(count<=1 && func.parameters.empty())
                inlineable.insert(func.definition);
 
-       SetForScope<FunctionDeclaration *> set(in_function, &func);
+       SetForScope<FunctionDeclaration *> set(current_function, &func);
+       return_count = 0;
        TraversingVisitor::visit(func);
 }
 
+void InlineableFunctionLocator::visit(Conditional &cond)
+{
+       TraversingVisitor::visit(cond);
+       inlineable.erase(current_function);
+}
+
+void InlineableFunctionLocator::visit(Iteration &iter)
+{
+       TraversingVisitor::visit(iter);
+       inlineable.erase(current_function);
+}
+
+void InlineableFunctionLocator::visit(Return &ret)
+{
+       TraversingVisitor::visit(ret);
+       if(return_count)
+               inlineable.erase(current_function);
+       ++return_count;
+}
+
+
+InlineContentInjector::InlineContentInjector():
+       source_func(0),
+       remap_names(false),
+       deps_only(false)
+{ }
+
+const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &target_func, Block &tgt_blk, const NodeList<Statement>::iterator &ins_pt, FunctionDeclaration &src)
+{
+       target_block = &tgt_blk;
+       source_func = &src;
+       for(NodeList<Statement>::iterator i=src.body.body.begin(); i!=src.body.body.end(); ++i)
+       {
+               r_inlined_statement = 0;
+               (*i)->visit(*this);
+               if(!r_inlined_statement)
+                       r_inlined_statement = (*i)->clone();
+
+               SetFlag set_remap(remap_names);
+               r_inlined_statement->visit(*this);
+               tgt_blk.body.insert(ins_pt, r_inlined_statement);
+       }
+
+       NodeReorderer().apply(stage, target_func, dependencies);
+
+       return r_result_name;
+}
+
+string InlineContentInjector::create_unused_name(const string &base, bool always_prefix)
+{
+       string result = base;
+       if(always_prefix || target_block->variables.count(result))
+               result = format("_%s_%s", source_func->name, base);
+       unsigned initial_size = result.size();
+       for(unsigned i=1; target_block->variables.count(result); ++i)
+       {
+               result.erase(initial_size);
+               result += format("_%d", i);
+       }
+       return result;
+}
+
+void InlineContentInjector::visit(VariableReference &var)
+{
+       if(remap_names)
+       {
+               map<string, VariableDeclaration *>::const_iterator i = variable_map.find(var.name);
+               if(i!=variable_map.end())
+                       var.name = i->second->name;
+       }
+       else if(var.declaration)
+       {
+               SetFlag set_deps(deps_only);
+               dependencies.insert(var.declaration);
+               var.declaration->visit(*this);
+       }
+}
+
+void InlineContentInjector::visit(InterfaceBlockReference &iface)
+{
+       if(!remap_names && iface.declaration)
+       {
+               SetFlag set_deps(deps_only);
+               dependencies.insert(iface.declaration);
+               iface.declaration->visit(*this);
+       }
+}
+
+void InlineContentInjector::visit(FunctionCall &call)
+{
+       if(!remap_names && call.declaration)
+               dependencies.insert(call.declaration);
+       TraversingVisitor::visit(call);
+}
+
+void InlineContentInjector::visit(VariableDeclaration &var)
+{
+       TraversingVisitor::visit(var);
+
+       if(var.type_declaration)
+       {
+               SetFlag set_deps(deps_only);
+               dependencies.insert(var.type_declaration);
+               var.type_declaration->visit(*this);
+       }
+
+       if(!remap_names && !deps_only)
+       {
+               RefPtr<VariableDeclaration> inlined_var = var.clone();
+               inlined_var->name = create_unused_name(var.name, false);
+               r_inlined_statement = inlined_var;
+
+               variable_map[var.name] = inlined_var.get();
+       }
+}
+
+void InlineContentInjector::visit(Return &ret)
+{
+       TraversingVisitor::visit(ret);
+
+       if(ret.expression)
+       {
+               /* Create a new variable to hold the return value of the inlined
+               function. */
+               r_result_name = create_unused_name("return", true);
+               RefPtr<VariableDeclaration> var = new VariableDeclaration;
+               var->source = ret.source;
+               var->line = ret.line;
+               var->type = source_func->return_type;
+               var->name = r_result_name;
+               var->init_expression = ret.expression->clone();
+               r_inlined_statement = var;
+       }
+}
+
 
 FunctionInliner::FunctionInliner():
-       extract_result(0)
+       current_function(0),
+       r_any_inlined(false)
 { }
 
-void FunctionInliner::apply(Stage &stage)
+bool FunctionInliner::apply(Stage &s)
 {
-       inlineable = InlineableFunctionLocator().apply(stage);
-       stage.content.visit(*this);
+       stage = &s;
+       inlineable = InlineableFunctionLocator().apply(s);
+       r_any_inlined = false;
+       s.content.visit(*this);
+       return r_any_inlined;
 }
 
 void FunctionInliner::visit_and_inline(RefPtr<Expression> &ptr)
 {
-       inline_result = 0;
+       r_inline_result = 0;
        ptr->visit(*this);
-       if(inline_result)
-               ptr = inline_result;
+       if(r_inline_result)
+       {
+               ptr = r_inline_result;
+               r_any_inlined = true;
+       }
+       r_inline_result = 0;
 }
 
 void FunctionInliner::visit(Block &block)
 {
-       if(extract_result)
-               --extract_result;
-
+       SetForScope<Block *> set_block(current_block, &block);
+       SetForScope<NodeList<Statement>::iterator> save_insert_point(insert_point, block.body.begin());
        for(NodeList<Statement>::iterator i=block.body.begin(); i!=block.body.end(); ++i)
        {
+               insert_point = i;
                (*i)->visit(*this);
-               if(extract_result)
-                       --extract_result;
        }
 }
 
 void FunctionInliner::visit(UnaryExpression &unary)
 {
        visit_and_inline(unary.expression);
-       inline_result = 0;
 }
 
 void FunctionInliner::visit(BinaryExpression &binary)
 {
        visit_and_inline(binary.left);
        visit_and_inline(binary.right);
-       inline_result = 0;
 }
 
 void FunctionInliner::visit(MemberAccess &memacc)
 {
        visit_and_inline(memacc.left);
-       inline_result = 0;
 }
 
 void FunctionInliner::visit(FunctionCall &call)
@@ -95,122 +238,371 @@ void FunctionInliner::visit(FunctionCall &call)
                visit_and_inline(*i);
 
        FunctionDeclaration *def = call.declaration;
-       if(def && def->definition!=def)
+       if(def)
                def = def->definition;
 
        if(def && inlineable.count(def))
        {
-               extract_result = 2;
-               def->visit(*this);
+               string result_name = InlineContentInjector().apply(*stage, *current_function, *current_block, insert_point, *def);
+
+               // This will later get removed by UnusedVariableRemover.
+               if(result_name.empty())
+                       result_name = "msp_unused_from_inline";
+
+               RefPtr<VariableReference> ref = new VariableReference;
+               ref->name = result_name;
+               r_inline_result = ref;
+
+               /* Inlined variables need to be resolved before this function can be
+               inlined further. */
+               inlineable.erase(current_function);
        }
-       else
-               inline_result = 0;
+}
+
+void FunctionInliner::visit(ExpressionStatement &expr)
+{
+       visit_and_inline(expr.expression);
 }
 
 void FunctionInliner::visit(VariableDeclaration &var)
 {
        if(var.init_expression)
                visit_and_inline(var.init_expression);
-       inline_result = 0;
 }
 
-void FunctionInliner::visit(Return &ret)
+void FunctionInliner::visit(FunctionDeclaration &func)
 {
-       TraversingVisitor::visit(ret);
+       SetForScope<FunctionDeclaration *> set_func(current_function, &func);
+       TraversingVisitor::visit(func);
+}
+
+void FunctionInliner::visit(Conditional &cond)
+{
+       visit_and_inline(cond.condition);
+       cond.body.visit(*this);
+}
+
+void FunctionInliner::visit(Iteration &iter)
+{
+       /* Visit the initialization statement before entering the loop body so the
+       inlined statements get inserted outside. */
+       if(iter.init_statement)
+               iter.init_statement->visit(*this);
 
-       if(extract_result)
-               inline_result = ret.expression->clone();
+       SetForScope<Block *> set_block(current_block, &iter.body);
+       /* Skip the condition and loop expression parts because they're not properly
+       inside the body block.  Inlining anything into them will require a more
+       comprehensive transformation. */
+       iter.body.visit(*this);
+}
+
+void FunctionInliner::visit(Return &ret)
+{
+       if(ret.expression)
+               visit_and_inline(ret.expression);
 }
 
 
-ConstantConditionEliminator::ConstantConditionEliminator():
-       record_only(false)
+ExpressionInliner::ExpressionInfo::ExpressionInfo():
+       expression(0),
+       assign_scope(0),
+       inline_point(0),
+       inner_oper(0),
+       outer_oper(0),
+       inline_on_rhs(false),
+       trivial(false),
+       available(true)
 { }
 
-void ConstantConditionEliminator::apply(Stage &stage)
+
+ExpressionInliner::ExpressionInliner():
+       r_ref_info(0),
+       r_any_inlined(false),
+       r_trivial(false),
+       mutating(false),
+       iteration_init(false),
+       iteration_body(0),
+       r_oper(0)
+{ }
+
+bool ExpressionInliner::apply(Stage &s)
 {
-       stage.content.visit(*this);
-       NodeRemover().apply(stage, nodes_to_remove);
+       s.content.visit(*this);
+       return r_any_inlined;
 }
 
-void ConstantConditionEliminator::visit(Block &block)
+void ExpressionInliner::visit_and_record(RefPtr<Expression> &ptr, const Operator *outer_oper, bool on_rhs)
 {
-       SetForScope<Block *> set_block(current_block, &block);
-       for(NodeList<Statement>::iterator i=block.body.begin(); i!=block.body.end(); ++i)
+       r_ref_info = 0;
+       ptr->visit(*this);
+       if(r_ref_info && r_ref_info->expression && r_ref_info->available)
        {
-               insert_point = i;
-               (*i)->visit(*this);
+               if(iteration_body && !r_ref_info->trivial)
+               {
+                       /* Don't inline non-trivial expressions which were assigned outside
+                       an iteration statement.  The iteration may run multiple times, which
+                       would cause the expression to also be evaluated multiple times. */
+                       Block *i = r_ref_info->assign_scope;
+                       for(; (i && i!=iteration_body); i=i->parent) ;
+                       if(!i)
+                               return;
+               }
+
+               r_ref_info->outer_oper = outer_oper;
+               if(r_ref_info->trivial)
+                       inline_expression(*r_ref_info->expression, ptr, outer_oper, r_ref_info->inner_oper, on_rhs);
+               else
+               {
+                       /* Record the inline point for a non-trivial expression but don't
+                       inline it yet.  It might turn out it shouldn't be inlined after all. */
+                       r_ref_info->inline_point = &ptr;
+                       r_ref_info->inline_on_rhs = on_rhs;
+               }
        }
+       r_ref_info = 0;
+}
+
+void ExpressionInliner::inline_expression(Expression &expr, RefPtr<Expression> &ptr, const Operator *outer_oper, const Operator *inner_oper, bool on_rhs)
+{
+       unsigned outer_precedence = (outer_oper ? outer_oper->precedence : 20);
+       unsigned inner_precedence = (inner_oper ? inner_oper->precedence : 0);
+
+       bool needs_parentheses = (inner_precedence>=outer_precedence);
+       if(inner_oper && inner_oper==outer_oper)
+               // Omit parentheses if the operator's natural grouping works out.
+               needs_parentheses = (inner_oper->assoc!=Operator::ASSOCIATIVE && on_rhs!=(inner_oper->assoc==Operator::RIGHT_TO_LEFT));
+
+       if(needs_parentheses)
+       {
+               RefPtr<ParenthesizedExpression> parexpr = new ParenthesizedExpression;
+               parexpr->expression = expr.clone();
+               ptr = parexpr;
+       }
+       else
+               ptr = expr.clone();
 
-       for(map<string, VariableDeclaration *>::const_iterator i=block.variables.begin(); i!=block.variables.end(); ++i)
-               variable_values.erase(i->second);
+       r_any_inlined = true;
 }
 
-void ConstantConditionEliminator::visit(UnaryExpression &unary)
+void ExpressionInliner::visit(Block &block)
 {
-       if(VariableReference *var = dynamic_cast<VariableReference *>(unary.expression.get()))
-               if(unary.oper=="++" || unary.oper=="--")
-                       variable_values.erase(var->declaration);
+       TraversingVisitor::visit(block);
+
+       for(map<VariableDeclaration *, ExpressionInfo>::iterator i=expressions.begin(); i!=expressions.end(); )
+       {
+               map<string, VariableDeclaration *>::iterator j = block.variables.find(i->first->name);
+               if(j!=block.variables.end() && j->second==i->first)
+               {
+                       if(i->second.expression && i->second.inline_point)
+                               inline_expression(*i->second.expression, *i->second.inline_point, i->second.outer_oper, i->second.inner_oper, i->second.inline_on_rhs);
+
+                       expressions.erase(i++);
+               }
+               else
+               {
+                       /* The expression was assigned in this block and may depend on local
+                       variables of the block.  If this is a conditionally executed block,
+                       the assignment might not always happen.  Mark the expression as not
+                       available to any outer blocks. */
+                       if(i->second.assign_scope==&block)
+                               i->second.available = false;
+
+                       ++i;
+               }
+       }
 }
 
-void ConstantConditionEliminator::visit(Assignment &assign)
+void ExpressionInliner::visit(VariableReference &var)
 {
-       variable_values.erase(assign.target_declaration);
+       if(var.declaration)
+       {
+               map<VariableDeclaration *, ExpressionInfo>::iterator i = expressions.find(var.declaration);
+               if(i!=expressions.end())
+               {
+                       /* If a non-trivial expression is referenced multiple times, don't
+                       inline it. */
+                       if(i->second.inline_point && !i->second.trivial)
+                               i->second.expression = 0;
+                       /* Mutating expressions are analogous to self-referencing assignments
+                       and prevent inlining. */
+                       if(mutating)
+                               i->second.expression = 0;
+                       r_ref_info = &i->second;
+               }
+       }
 }
 
-void ConstantConditionEliminator::visit(VariableDeclaration &var)
+void ExpressionInliner::visit(MemberAccess &memacc)
 {
-       if((var.constant || current_block->parent) && var.init_expression)
-               variable_values[&var] = var.init_expression.get();
+       visit_and_record(memacc.left, memacc.oper, false);
+       r_oper = memacc.oper;
+       r_trivial = false;
 }
 
-void ConstantConditionEliminator::visit(Conditional &cond)
+void ExpressionInliner::visit(UnaryExpression &unary)
+{
+       SetFlag set_target(mutating, mutating || unary.oper->token[1]=='+' || unary.oper->token[1]=='-');
+       visit_and_record(unary.expression, unary.oper, false);
+       r_oper = unary.oper;
+       r_trivial = false;
+}
+
+void ExpressionInliner::visit(BinaryExpression &binary)
 {
-       if(!record_only)
+       visit_and_record(binary.left, binary.oper, false);
+       {
+               SetFlag clear_target(mutating, false);
+               visit_and_record(binary.right, binary.oper, true);
+       }
+       r_oper = binary.oper;
+       r_trivial = false;
+}
+
+void ExpressionInliner::visit(Assignment &assign)
+{
+       {
+               SetFlag set_target(mutating);
+               visit_and_record(assign.left, assign.oper, false);
+       }
+       r_oper = 0;
+       visit_and_record(assign.right, assign.oper, true);
+
+       if(assign.target_declaration)
        {
-               ExpressionEvaluator eval(variable_values);
-               cond.condition->visit(eval);
-               if(eval.is_result_valid())
+               map<VariableDeclaration *, ExpressionInfo>::iterator i = expressions.find(assign.target_declaration);
+               if(i!=expressions.end())
                {
-                       Block &block = (eval.get_result() ? cond.body : cond.else_body);
-                       current_block->body.splice(insert_point, block.body);
-                       nodes_to_remove.insert(&cond);
-                       return;
+                       /* Self-referencing assignments can't be inlined without additional
+                       work.  Just clear any previous expression. */
+                       i->second.expression = (assign.self_referencing ? 0 : assign.right.get());
+                       i->second.assign_scope = current_block;
+                       i->second.inline_point = 0;
+                       i->second.inner_oper = r_oper;
+                       i->second.available = true;
                }
        }
 
+       r_oper = assign.oper;
+       r_trivial = false;
+}
+
+void ExpressionInliner::visit(FunctionCall &call)
+{
+       for(NodeArray<Expression>::iterator i=call.arguments.begin(); i!=call.arguments.end(); ++i)
+               visit_and_record(*i, 0, false);
+       r_oper = 0;
+       r_trivial = false;
+}
+
+void ExpressionInliner::visit(VariableDeclaration &var)
+{
+       r_oper = 0;
+       r_trivial = true;
+       if(var.init_expression)
+               visit_and_record(var.init_expression, 0, false);
+
+       bool constant = var.constant;
+       if(constant && var.layout)
+       {
+               for(vector<Layout::Qualifier>::const_iterator i=var.layout->qualifiers.begin(); (constant && i!=var.layout->qualifiers.end()); ++i)
+                       constant = (i->name!="constant_id");
+       }
+
+       /* Only inline global variables if they're constant and have trivial
+       initializers.  Non-constant variables could change in ways which are hard to
+       analyze and non-trivial expressions could be expensive to inline.  */
+       if((current_block->parent || (constant && r_trivial)) && var.interface.empty())
+       {
+               ExpressionInfo &info = expressions[&var];
+               /* Assume variables declared in an iteration initialization statement
+               will have their values change throughout the iteration. */
+               info.expression = (iteration_init ? 0 : var.init_expression.get());
+               info.assign_scope = current_block;
+               info.inner_oper = r_oper;
+               info.trivial = r_trivial;
+       }
+}
+
+void ExpressionInliner::visit(Conditional &cond)
+{
+       visit_and_record(cond.condition, 0, false);
+       cond.body.visit(*this);
+}
+
+void ExpressionInliner::visit(Iteration &iter)
+{
+       SetForScope<Block *> set_block(current_block, &iter.body);
+       if(iter.init_statement)
+       {
+               SetFlag set_init(iteration_init);
+               iter.init_statement->visit(*this);
+       }
+
+       SetForScope<Block *> set_body(iteration_body, &iter.body);
+       if(iter.condition)
+               iter.condition->visit(*this);
+       iter.body.visit(*this);
+       if(iter.loop_expression)
+               iter.loop_expression->visit(*this);
+}
+
+void ExpressionInliner::visit(Return &ret)
+{
+       if(ret.expression)
+               visit_and_record(ret.expression, 0, false);
+}
+
+
+void ConstantConditionEliminator::apply(Stage &stage)
+{
+       stage.content.visit(*this);
+       NodeRemover().apply(stage, nodes_to_remove);
+}
+
+void ConstantConditionEliminator::visit(Block &block)
+{
+       SetForScope<Block *> set_block(current_block, &block);
+       for(NodeList<Statement>::iterator i=block.body.begin(); i!=block.body.end(); ++i)
+       {
+               insert_point = i;
+               (*i)->visit(*this);
+       }
+}
+
+void ConstantConditionEliminator::visit(Conditional &cond)
+{
+       ExpressionEvaluator eval;
+       cond.condition->visit(eval);
+       if(eval.is_result_valid())
+       {
+               Block &block = (eval.get_result() ? cond.body : cond.else_body);
+               current_block->body.splice(insert_point, block.body);
+               nodes_to_remove.insert(&cond);
+               return;
+       }
+
        TraversingVisitor::visit(cond);
 }
 
 void ConstantConditionEliminator::visit(Iteration &iter)
 {
-       if(!record_only)
+       if(iter.condition)
        {
-               if(iter.condition)
+               /* If the loop condition is always false on the first iteration, the
+               entire loop can be removed */
+               ExpressionEvaluator::ValueMap values;
+               if(VariableDeclaration *var = dynamic_cast<VariableDeclaration *>(iter.init_statement.get()))
+                       values[var] = var->init_expression.get();
+               ExpressionEvaluator eval(values);
+               iter.condition->visit(eval);
+               if(eval.is_result_valid() && !eval.get_result())
                {
-                       /* If the loop condition is always false on the first iteration, the
-                       entire loop can be removed */
-                       if(iter.init_statement)
-                               iter.init_statement->visit(*this);
-                       ExpressionEvaluator eval(variable_values);
-                       iter.condition->visit(eval);
-                       if(eval.is_result_valid() && !eval.get_result())
-                       {
-                               nodes_to_remove.insert(&iter);
-                               return;
-                       }
+                       nodes_to_remove.insert(&iter);
+                       return;
                }
-
-               /* Record all assignments that occur inside the loop body so those
-               variables won't be considered as constant */
-               SetFlag set_record(record_only);
-               TraversingVisitor::visit(iter);
        }
 
        TraversingVisitor::visit(iter);
-
-       if(VariableDeclaration *init_decl = dynamic_cast<VariableDeclaration *>(iter.init_statement.get()))
-               variable_values.erase(init_decl);
 }
 
 
@@ -221,11 +613,79 @@ UnusedVariableRemover::VariableInfo::VariableInfo():
 { }
 
 
+bool UnusedTypeRemover::apply(Stage &stage)
+{
+       stage.content.visit(*this);
+       NodeRemover().apply(stage, unused_nodes);
+       return !unused_nodes.empty();
+}
+
+void UnusedTypeRemover::visit(Literal &literal)
+{
+       unused_nodes.erase(literal.type);
+}
+
+void UnusedTypeRemover::visit(UnaryExpression &unary)
+{
+       unused_nodes.erase(unary.type);
+       TraversingVisitor::visit(unary);
+}
+
+void UnusedTypeRemover::visit(BinaryExpression &binary)
+{
+       unused_nodes.erase(binary.type);
+       TraversingVisitor::visit(binary);
+}
+
+void UnusedTypeRemover::visit(FunctionCall &call)
+{
+       unused_nodes.erase(call.type);
+       TraversingVisitor::visit(call);
+}
+
+void UnusedTypeRemover::visit(BasicTypeDeclaration &type)
+{
+       if(type.base_type)
+               unused_nodes.erase(type.base_type);
+       unused_nodes.insert(&type);
+}
+
+void UnusedTypeRemover::visit(ImageTypeDeclaration &type)
+{
+       if(type.base_type)
+               unused_nodes.erase(type.base_type);
+       unused_nodes.insert(&type);
+}
+
+void UnusedTypeRemover::visit(StructDeclaration &strct)
+{
+       unused_nodes.insert(&strct);
+       TraversingVisitor::visit(strct);
+}
+
+void UnusedTypeRemover::visit(VariableDeclaration &var)
+{
+       unused_nodes.erase(var.type_declaration);
+}
+
+void UnusedTypeRemover::visit(InterfaceBlock &iface)
+{
+       unused_nodes.erase(iface.type_declaration);
+}
+
+void UnusedTypeRemover::visit(FunctionDeclaration &func)
+{
+       unused_nodes.erase(func.return_type_declaration);
+       TraversingVisitor::visit(func);
+}
+
+
 UnusedVariableRemover::UnusedVariableRemover():
        aggregate(0),
-       assignment(0),
+       r_assignment(0),
        assignment_target(false),
-       assign_to_subscript(false)
+       r_assign_to_subfield(false),
+       r_side_effects(false)
 { }
 
 bool UnusedVariableRemover::apply(Stage &stage)
@@ -235,8 +695,22 @@ bool UnusedVariableRemover::apply(Stage &stage)
        BlockVariableMap &global_variables = variables.back();
        for(BlockVariableMap::iterator i=global_variables.begin(); i!=global_variables.end(); ++i)
        {
-               if(i->first->interface=="out" && (stage.type==Stage::FRAGMENT || i->first->linked_declaration || !i->first->name.compare(0, 3, "gl_")))
+               string interface = i->first->interface;
+               bool linked = i->first->linked_declaration;
+               map<VariableDeclaration *, Node *>::iterator j = aggregates.find(i->first);
+               if(j!=aggregates.end())
+                       if(InterfaceBlock *iface = dynamic_cast<InterfaceBlock *>(j->second))
+                       {
+                               interface = iface->interface;
+                               linked = iface->linked_block;
+                       }
+
+               /* Don't remove output variables which are used by the next stage or the
+               graphics API. */
+               if(interface=="out" && (stage.type==Stage::FRAGMENT || linked || !i->first->name.compare(0, 3, "gl_")))
                        continue;
+
+               // Mark other unreferenced global variables as unused.
                if(!i->second.referenced)
                {
                        unused_nodes.insert(i->first);
@@ -259,25 +733,40 @@ void UnusedVariableRemover::visit(VariableReference &var)
        if(var.declaration && !assignment_target)
        {
                VariableInfo &var_info = variables.back()[var.declaration];
-               var_info.assignments.clear();
+               // Previous assignments are used by this reference.
+               clear_assignments(var_info, false);
                var_info.referenced = true;
        }
 }
 
+void UnusedVariableRemover::visit(InterfaceBlockReference &iface)
+{
+       unused_nodes.erase(iface.declaration);
+}
+
 void UnusedVariableRemover::visit(MemberAccess &memacc)
 {
+       if(assignment_target)
+               r_assign_to_subfield = true;
        TraversingVisitor::visit(memacc);
        unused_nodes.erase(memacc.declaration);
 }
 
+void UnusedVariableRemover::visit(UnaryExpression &unary)
+{
+       TraversingVisitor::visit(unary);
+       if(unary.oper->token[1]=='+' || unary.oper->token[1]=='-')
+               r_side_effects = true;
+}
+
 void UnusedVariableRemover::visit(BinaryExpression &binary)
 {
-       if(binary.oper=="[")
+       if(binary.oper->token[0]=='[')
        {
                if(assignment_target)
-                       assign_to_subscript = true;
+                       r_assign_to_subfield = true;
                binary.left->visit(*this);
-               SetForScope<bool> set(assignment_target, false);
+               SetFlag set(assignment_target, false);
                binary.right->visit(*this);
        }
        else
@@ -287,17 +776,27 @@ void UnusedVariableRemover::visit(BinaryExpression &binary)
 void UnusedVariableRemover::visit(Assignment &assign)
 {
        {
-               assign_to_subscript = false;
-               SetForScope<bool> set(assignment_target, !assign.self_referencing);
+               SetFlag set(assignment_target, !assign.self_referencing);
                assign.left->visit(*this);
        }
        assign.right->visit(*this);
-       assignment = &assign;
+       r_assignment = &assign;
+       r_side_effects = true;
+}
+
+void UnusedVariableRemover::visit(FunctionCall &call)
+{
+       TraversingVisitor::visit(call);
+       /* Treat function calls as having side effects so expression statements
+       consisting of nothing but a function call won't be optimized away. */
+       r_side_effects = true;
 }
 
 void UnusedVariableRemover::record_assignment(VariableDeclaration &var, Node &node, bool chained)
 {
        VariableInfo &var_info = variables.back()[&var];
+       /* An assignment which completely replaces the value of the variable causes
+       any previous unreferenced assignments to be unused. */
        if(!chained)
                clear_assignments(var_info, true);
        var_info.assignments.push_back(&node);
@@ -316,16 +815,19 @@ void UnusedVariableRemover::clear_assignments(VariableInfo &var_info, bool mark_
 
 void UnusedVariableRemover::visit(ExpressionStatement &expr)
 {
-       assignment = 0;
+       r_assignment = 0;
+       r_assign_to_subfield = false;
+       r_side_effects = false;
        TraversingVisitor::visit(expr);
-       if(assignment && assignment->target_declaration)
-               record_assignment(*assignment->target_declaration, expr, (assignment->self_referencing || assign_to_subscript));
+       if(r_assignment && r_assignment->target_declaration)
+               record_assignment(*r_assignment->target_declaration, expr, (r_assignment->self_referencing || r_assign_to_subfield));
+       if(!r_side_effects)
+               unused_nodes.insert(&expr);
 }
 
 void UnusedVariableRemover::visit(StructDeclaration &strct)
 {
        SetForScope<Node *> set(aggregate, &strct);
-       unused_nodes.insert(&strct);
        TraversingVisitor::visit(strct);
 }
 
@@ -339,7 +841,6 @@ void UnusedVariableRemover::visit(VariableDeclaration &var)
                if(var.init_expression)
                        record_assignment(var, *var.init_expression, false);
        }
-       unused_nodes.erase(var.type_declaration);
        TraversingVisitor::visit(var);
 }
 
@@ -347,7 +848,7 @@ void UnusedVariableRemover::visit(InterfaceBlock &iface)
 {
        SetForScope<Node *> set(aggregate, &iface);
        unused_nodes.insert(&iface);
-       TraversingVisitor::visit(iface);
+       iface.struct_declaration->members.visit(*this);
 }
 
 void UnusedVariableRemover::visit(FunctionDeclaration &func)
@@ -359,10 +860,18 @@ void UnusedVariableRemover::visit(FunctionDeclaration &func)
        func.body.visit(*this);
 
        BlockVariableMap &block_variables = variables.back();
+
+       /* Mark global variables as conditionally assigned so assignments in other
+       functions won't be removed. */
        for(BlockVariableMap::iterator i=block_variables.begin(); i!=block_variables.end(); ++i)
-               i->second.conditionally_assigned = true;
+               if(!i->second.local)
+                       i->second.conditionally_assigned = true;
+
+       /* Always treat function parameters as referenced.  Removing unused
+       parameters is not currently supported. */
        for(NodeArray<VariableDeclaration>::iterator i=func.parameters.begin(); i!=func.parameters.end(); ++i)
                block_variables[i->get()].referenced = true;
+
        merge_down_variables();
 }
 
@@ -376,7 +885,9 @@ void UnusedVariableRemover::merge_down_variables()
                {
                        if(!i->second.referenced)
                                unused_nodes.insert(i->first);
-                       clear_assignments(i->second, i->first->interface!="out");
+                       /* Any unreferenced assignments when a variable runs out of scope
+                       become unused. */
+                       clear_assignments(i->second, true);
                        continue;
                }
 
@@ -385,6 +896,7 @@ void UnusedVariableRemover::merge_down_variables()
                        parent_variables.insert(*i);
                else
                {
+                       // Merge a non-local variable's state into the parent scope.
                        if(i->second.referenced || !i->second.conditionally_assigned)
                                clear_assignments(j->second, !i->second.referenced);
                        j->second.conditionally_assigned = i->second.conditionally_assigned;
@@ -405,20 +917,25 @@ void UnusedVariableRemover::visit(Conditional &cond)
        swap(variables.back(), if_variables);
        cond.else_body.visit(*this);
 
+       // Combine variables from both branches.
        BlockVariableMap &else_variables = variables.back();
        for(BlockVariableMap::iterator i=else_variables.begin(); i!=else_variables.end(); ++i)
        {
                BlockVariableMap::iterator j = if_variables.find(i->first);
                if(j!=if_variables.end())
                {
+                       // The variable was found in both branches.
                        i->second.assignments.insert(i->second.assignments.end(), j->second.assignments.begin(), j->second.assignments.end());
                        i->second.conditionally_assigned |= j->second.conditionally_assigned;
                        if_variables.erase(j);
                }
                else
+                       // Mark variables found in only one branch as conditionally assigned.
                        i->second.conditionally_assigned = true;
        }
 
+       /* Move variables which were only used in the if block into the combined
+       block. */
        for(BlockVariableMap::iterator i=if_variables.begin(); i!=if_variables.end(); ++i)
        {
                i->second.conditionally_assigned = true;
@@ -432,12 +949,6 @@ void UnusedVariableRemover::visit(Iteration &iter)
 {
        variables.push_back(BlockVariableMap());
        TraversingVisitor::visit(iter);
-
-       BlockVariableMap &block_variables = variables.back();
-       for(BlockVariableMap::iterator i=block_variables.begin(); i!=block_variables.end(); ++i)
-               if(!i->second.local && i->second.referenced)
-                       i->second.assignments.clear();
-
        merge_down_variables();
 }