Fix various issues with constant condition elimination

It used to optimize based on the initial values of variables, ignoring
the fact that they may change between loop iterations.  A stop-gap fix
was implemented in 8e14c29, but it prevented optimizations of some
conditionals that could safely have been removed.  It also caused the
optimizer to ignore any assignments made to any variables after
declaration when evaluating the loop condition, potentially causing
entire loops to be erroneously removed.  Finally, unary expressions were
not being handled so ++ and -- operators did not mark the variable as
modified.

diff --git a/source/programcompiler.cpp b/source/programcompiler.cpp
index a1be91ca4f575228fe2f48ccb374fb0097c6dccb..1f014f72c654d3e678f706cfff28f9d5173407f4 100644 (file)
--- a/source/programcompiler.cpp
+++ b/source/programcompiler.cpp
@@ -1210,14 +1210,12 @@ void ProgramCompiler::FunctionInliner::visit(Return &ret)
 
 ProgramCompiler::ExpressionEvaluator::ExpressionEvaluator():
        variable_values(0),
-       const_only(false),
        result(0.0f),
        result_valid(false)
 { }
 
-ProgramCompiler::ExpressionEvaluator::ExpressionEvaluator(const ValueMap *v, bool c):
-       variable_values(v),
-       const_only(c),
+ProgramCompiler::ExpressionEvaluator::ExpressionEvaluator(const ValueMap &v):
+       variable_values(&v),
        result(0.0f),
        result_valid(false)
 { }
@@ -1242,8 +1240,6 @@ void ProgramCompiler::ExpressionEvaluator::visit(VariableReference &var)
 {
        if(!var.declaration)
                return;
-       if(const_only && !var.declaration->constant)
-               return;
 
        if(variable_values)
        {
@@ -1304,7 +1300,7 @@ void ProgramCompiler::ExpressionEvaluator::visit(BinaryExpression &binary)
 
 ProgramCompiler::ConstantConditionEliminator::ConstantConditionEliminator():
        scope_level(0),
-       in_loop(false)
+       record_only(false)
 { }
 
 void ProgramCompiler::ConstantConditionEliminator::visit(Block &block)
@@ -1316,6 +1312,13 @@ void ProgramCompiler::ConstantConditionEliminator::visit(Block &block)
                variable_values.erase(i->second);
 }
 
+void ProgramCompiler::ConstantConditionEliminator::visit(UnaryExpression &unary)
+{
+       if(VariableReference *var = dynamic_cast<VariableReference *>(unary.expression.get()))
+               if(unary.oper=="++" || unary.oper=="--")
+                       variable_values.erase(var->declaration);
+}
+
 void ProgramCompiler::ConstantConditionEliminator::visit(Assignment &assign)
 {
        variable_values.erase(assign.target_declaration);
@@ -1329,30 +1332,49 @@ void ProgramCompiler::ConstantConditionEliminator::visit(VariableDeclaration &va
 
 void ProgramCompiler::ConstantConditionEliminator::visit(Conditional &cond)
 {
-       ExpressionEvaluator eval(&variable_values, in_loop);
-       cond.condition->visit(eval);
-       if(eval.result_valid)
-               flatten_block(eval.result ? cond.body : cond.else_body);
-       else
-               TraversingVisitor::visit(cond);
+       if(!record_only)
+       {
+               ExpressionEvaluator eval(variable_values);
+               cond.condition->visit(eval);
+               if(eval.result_valid)
+               {
+                       flatten_block(eval.result ? cond.body : cond.else_body);
+                       return;
+               }
+       }
+
+       TraversingVisitor::visit(cond);
 }
 
 void ProgramCompiler::ConstantConditionEliminator::visit(Iteration &iter)
 {
-       // XXX Should this not visit init_statement first?
-       if(iter.condition)
+       if(!record_only)
        {
-               ExpressionEvaluator eval(0, in_loop);
-               iter.condition->visit(eval);
-               if(eval.result_valid && !eval.result)
+               if(iter.condition)
                {
-                       remove_node = true;
-                       return;
+                       /* 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.result_valid && !eval.result)
+                       {
+                               remove_node = true;
+                               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);
        }
 
-       SetFlag set_loop(in_loop);
        TraversingVisitor::visit(iter);
+
+       if(VariableDeclaration *init_decl = dynamic_cast<VariableDeclaration *>(iter.init_statement.get()))
+               variable_values.erase(init_decl);
 }
 
 

diff --git a/source/programcompiler.h b/source/programcompiler.h
index 2c8b33b6f905b726ed6eea6aa5a0230dee27f970..461fac9a2e649b1fb3f2597da5ebed6759b218ff 100644 (file)
--- a/source/programcompiler.h
+++ b/source/programcompiler.h
@@ -226,12 +226,11 @@ private:
                typedef std::map<ProgramSyntax::VariableDeclaration *, ProgramSyntax::Expression *> ValueMap;
 
                const ValueMap *variable_values;
-               bool const_only;
                float result;
                bool result_valid;
 
                ExpressionEvaluator();
-               ExpressionEvaluator(const ValueMap *, bool);
+               ExpressionEvaluator(const ValueMap &);
 
                using ProgramSyntax::NodeVisitor::visit;
                virtual void visit(ProgramSyntax::Literal &);
@@ -244,13 +243,14 @@ private:
        struct ConstantConditionEliminator: BlockModifier
        {
                unsigned scope_level;
-               bool in_loop;
+               bool record_only;
                ExpressionEvaluator::ValueMap variable_values;
 
                ConstantConditionEliminator();
 
                using Visitor::visit;
                virtual void visit(ProgramSyntax::Block &);
+               virtual void visit(ProgramSyntax::UnaryExpression &);
                virtual void visit(ProgramSyntax::Assignment &);
                virtual void visit(ProgramSyntax::VariableDeclaration &);
                virtual void visit(ProgramSyntax::Conditional &);