From: Mikko Rasa Date: Sat, 25 May 2019 21:00:54 +0000 (+0300) Subject: Fix various issues with constant condition elimination X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=aa9873652630db493b5bd9faee4117e9c30ef226;p=libs%2Fgl.git 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 a1be91ca..1f014f72 100644 --- 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(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(iter.init_statement.get())) + variable_values.erase(init_decl); } diff --git a/source/programcompiler.h b/source/programcompiler.h index 2c8b33b6..461fac9a 100644 --- a/source/programcompiler.h +++ b/source/programcompiler.h @@ -226,12 +226,11 @@ private: typedef std::map 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 &);