From 91f96c299e765d517dd705db6512092f91dff37a Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 26 Mar 2019 17:35:26 +0100 Subject: [PATCH] More logic about control flow with continue and about finalize. Remove BlockScope. --- .../optimiser/RedundantAssignEliminator.cpp | 74 +++++++++++------- libyul/optimiser/RedundantAssignEliminator.h | 34 ++------- .../redundantAssignEliminator/for_break.yul | 26 ++++--- .../for_continue.yul | 30 ++++---- .../for_continue_2.yul | 26 ++++--- .../for_continue_3.yul | 23 +++--- .../for_decl_inside_break_continue.yul | 44 +++++++++++ .../for_multi_break.yul | 74 ++++++++++++++++++ .../redundantAssignEliminator/for_nested.yul | 76 +++++++++++++++++++ .../for_stmnts_after_break_continue.yul | 52 +++++++++++++ 10 files changed, 356 insertions(+), 103 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/redundantAssignEliminator/for_decl_inside_break_continue.yul create mode 100644 test/libyul/yulOptimizerTests/redundantAssignEliminator/for_multi_break.yul create mode 100644 test/libyul/yulOptimizerTests/redundantAssignEliminator/for_nested.yul create mode 100644 test/libyul/yulOptimizerTests/redundantAssignEliminator/for_stmnts_after_break_continue.yul diff --git a/libyul/optimiser/RedundantAssignEliminator.cpp b/libyul/optimiser/RedundantAssignEliminator.cpp index 857ddd9d4..e679be540 100644 --- a/libyul/optimiser/RedundantAssignEliminator.cpp +++ b/libyul/optimiser/RedundantAssignEliminator.cpp @@ -97,31 +97,30 @@ void RedundantAssignEliminator::operator()(FunctionDefinition const& _functionDe { std::set outerDeclaredVariables; TrackedAssignments outerAssignments; + ForLoopInfo forLoopInfo; swap(m_declaredVariables, outerDeclaredVariables); swap(m_assignments, outerAssignments); + swap(m_forLoopInfo, forLoopInfo); (*this)(_functionDefinition.body); for (auto const& param: _functionDefinition.parameters) - { - changeUndecidedTo(param.name, State::Unused); - finalize(param.name); - } + finalize(param.name, State::Unused); for (auto const& retParam: _functionDefinition.returnVariables) - { - changeUndecidedTo(retParam.name, State::Used); - finalize(retParam.name); - } + finalize(retParam.name, State::Used); swap(m_declaredVariables, outerDeclaredVariables); swap(m_assignments, outerAssignments); + swap(m_forLoopInfo, forLoopInfo); } void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) { - // This will set all variables that are declared in this - // block to "unused" when it is destroyed. - BlockScope scope(*this); + ForLoopInfo outerForLoopInfo; + swap(outerForLoopInfo, m_forLoopInfo); + + set outerDeclaredVariables; + swap(m_declaredVariables, outerDeclaredVariables); // We need to visit the statements directly because of the // scoping rules. @@ -133,9 +132,6 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) visit(*_forLoop.condition); - ForLoopInfo outerForLoopInfo; - swap(outerForLoopInfo, m_forLoopInfo); - TrackedAssignments zeroRuns{m_assignments}; (*this)(_forLoop.body); @@ -150,6 +146,7 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) (*this)(_forLoop.body); merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts)); + m_forLoopInfo.pendingContinueStmts.clear(); (*this)(_forLoop.post); visit(*_forLoop.condition); @@ -158,28 +155,39 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) merge(m_assignments, move(oneRun)); merge(m_assignments, move(zeroRuns)); merge(m_assignments, move(m_forLoopInfo.pendingBreakStmts)); + m_forLoopInfo.pendingBreakStmts.clear(); - // Oestore potential outer for-loop states. + for (auto const& var: m_declaredVariables) + finalize(var, State::Unused); + swap(m_declaredVariables, outerDeclaredVariables); + + // Restore potential outer for-loop states. swap(m_forLoopInfo, outerForLoopInfo); } void RedundantAssignEliminator::operator()(Break const&) { - m_forLoopInfo.pendingBreakStmts.push_back(m_assignments); + m_forLoopInfo.pendingBreakStmts.emplace_back(move(m_assignments)); + m_assignments.clear(); } void RedundantAssignEliminator::operator()(Continue const&) { - m_forLoopInfo.pendingContinueStmts.push_back(m_assignments); + m_forLoopInfo.pendingContinueStmts.emplace_back(move(m_assignments)); + m_assignments.clear(); } void RedundantAssignEliminator::operator()(Block const& _block) { - // This will set all variables that are declared in this - // block to "unused" when it is destroyed. - BlockScope scope(*this); + set outerDeclaredVariables; + swap(m_declaredVariables, outerDeclaredVariables); ASTWalker::operator()(_block); + + for (auto const& var: m_declaredVariables) + finalize(var, State::Unused); + + swap(m_declaredVariables, outerDeclaredVariables); } void RedundantAssignEliminator::run(Dialect const& _dialect, Block& _ast) @@ -239,21 +247,35 @@ void RedundantAssignEliminator::merge(TrackedAssignments& _target, vectorvalue}.movable()) + State const state = assignment.second == State::Undecided ? _finalState : assignment.second; + + if (state == State::Unused && MovableChecker{*m_dialect, *assignment.first->value}.movable()) // TODO the only point where we actually need this // to be a set is for the for loop m_pendingRemovals.insert(assignment.first); } - m_assignments.erase(_variable); + _assignments.erase(_variable); } void AssignmentRemover::operator()(Block& _block) diff --git a/libyul/optimiser/RedundantAssignEliminator.h b/libyul/optimiser/RedundantAssignEliminator.h index ca6050369..3165d0335 100644 --- a/libyul/optimiser/RedundantAssignEliminator.h +++ b/libyul/optimiser/RedundantAssignEliminator.h @@ -137,33 +137,6 @@ private: Value m_value = Undecided; }; - /** - * Takes care about storing the list of declared variables and - * sets them to "unused" when it is destroyed. - */ - class BlockScope - { - public: - explicit BlockScope(RedundantAssignEliminator& _rae): m_rae(_rae) - { - swap(m_rae.m_declaredVariables, m_outerDeclaredVariables); - } - ~BlockScope() - { - // This should actually store all declared variables - // into a different mapping - for (auto const& var: m_rae.m_declaredVariables) - m_rae.changeUndecidedTo(var, State::Unused); - for (auto const& var: m_rae.m_declaredVariables) - m_rae.finalize(var); - swap(m_rae.m_declaredVariables, m_outerDeclaredVariables); - } - - private: - RedundantAssignEliminator& m_rae; - std::set m_outerDeclaredVariables; - }; - // TODO check that this does not cause nondeterminism! // This could also be a pseudo-map from state to assignment. using TrackedAssignments = std::map>; @@ -174,7 +147,12 @@ private: static void merge(TrackedAssignments& _target, TrackedAssignments&& _source); static void merge(TrackedAssignments& _target, std::vector&& _source); void changeUndecidedTo(YulString _variable, State _newState); - void finalize(YulString _variable); + /// Called when a variable goes out of scope. Sets the state of all still undecided + /// assignments to the final state. In this case, this also applies to pending + /// break and continue TrackedAssignments. + void finalize(YulString _variable, State _finalState); + /// Helper function for the above. + void finalize(TrackedAssignments& _assignments, YulString _variable, State _finalState); Dialect const* m_dialect; std::set m_declaredVariables; diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul index 28d8b01b3..8919a0309 100644 --- a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul @@ -1,20 +1,22 @@ { - // Cannot be removed, because we might run the loop only once - let x := 1 - for { } calldataload(0) { } - { - if callvalue() { - x := 2 // is preserved because of break stmt below. - break - } - x := 3 - } - mstore(x, 0x42) + let x + // Cannot be removed, because we might skip the loop + x := 1 + for { } calldataload(0) { } + { + if callvalue() { + x := 2 // is preserved because of break stmt below. + break + } + x := 3 + } + mstore(x, 0x42) } // ---- // redundantAssignEliminator // { -// let x := 1 +// let x +// x := 1 // for { // } // calldataload(0) diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul index afa9f3187..e21b76a77 100644 --- a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul @@ -1,21 +1,24 @@ { - // Cannot be removed, because we might run the loop only once - let x := 1 - for { } calldataload(0) { } - { - x := 2 // Will not be removed as if-condition can be false. - if callvalue() { - x := 3 - continue - } - mstore(x, 2) - } - x := 3 + let x + // Can be removed, because x is reassigned after the loop + x := 1 + for { } calldataload(0) { } + { + x := 2 // Will not be removed as if-condition can be false. + if callvalue() { + // This can be removed because x is overwritten both after the + // loop at at the start of the next iteration. + x := 3 + continue + } + mstore(x, 2) + } + x := 3 } // ---- // redundantAssignEliminator // { -// let x := 1 +// let x // for { // } // calldataload(0) @@ -25,7 +28,6 @@ // x := 2 // if callvalue() // { -// x := 3 // continue // } // mstore(x, 2) diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul index f71862250..ef1cac231 100644 --- a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul @@ -1,20 +1,22 @@ { - // Cannot be removed, because we might run the loop only once - let x := 1 - for { } calldataload(0) { } - { - if callvalue() { - x := 2 // is preserved because of continue stmt below. - continue - } - x := 3 - } - mstore(x, 0x42) + let x + // Cannot be removed, because we might skip the loop + x := 1 + for { } calldataload(0) { } + { + if callvalue() { + x := 2 // is preserved because of continue stmt below. + continue + } + x := 3 + } + mstore(x, 0x42) } // ---- // redundantAssignEliminator // { -// let x := 1 +// let x +// x := 1 // for { // } // calldataload(0) diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul index dd3bb6c63..246d97248 100644 --- a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul @@ -1,19 +1,20 @@ { - // Cannot be removed, because we might run the loop only once - let x := 1 - for { } calldataload(0) { mstore(x, 0x42) } - { - if callvalue() { - x := 2 // is preserved because of continue stmt below. - continue - } - x := 3 - } + let x + // Can be removed, because x is not used after the loop. + x := 1 + for { } calldataload(0) { mstore(x, 0x42) } + { + if callvalue() { + x := 2 // is preserved because of continue stmt below. + continue + } + x := 3 + } } // ---- // redundantAssignEliminator // { -// let x := 1 +// let x // for { // } // calldataload(0) diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_decl_inside_break_continue.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_decl_inside_break_continue.yul new file mode 100644 index 000000000..bdf6a8ae9 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_decl_inside_break_continue.yul @@ -0,0 +1,44 @@ +{ + let x := 1 + for { } calldataload(0) { } + { + // This will go out of scope at the end of the block, + // but the continue/break statements still refer to it. + { + let y := 9 + if callvalue() { + y := 2 // will be removed + break + } + if eq(callvalue(), 3) { + y := 12 // will be removed + continue + } + } + } + mstore(x, 0x42) +} +// ---- +// redundantAssignEliminator +// { +// let x := 1 +// for { +// } +// calldataload(0) +// { +// } +// { +// { +// let y := 9 +// if callvalue() +// { +// break +// } +// if eq(callvalue(), 3) +// { +// continue +// } +// } +// } +// mstore(x, 0x42) +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_multi_break.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_multi_break.yul new file mode 100644 index 000000000..3a5212641 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_multi_break.yul @@ -0,0 +1,74 @@ +{ + let x := 1 + let y := 1 + let z := 1 + for { } calldataload(0) { mstore(x, 2) mstore(z, 2) } + { + y := 3 + switch callvalue() + case 0 { + x := 2 + y := 2 + z := 2 + break + } + case 1 { + x := 3 + y := 3 + z := 3 + continue + } + case 2 { + x := 4 + y := 4 + z := 4 + break + } + case 3 { + x := 5 + y := 5 + z := 5 + continue + } + mstore(y, 9) + } + mstore(x, 0x42) +} +// ---- +// redundantAssignEliminator +// { +// let x := 1 +// let y := 1 +// let z := 1 +// for { +// } +// calldataload(0) +// { +// mstore(x, 2) +// mstore(z, 2) +// } +// { +// y := 3 +// switch callvalue() +// case 0 { +// x := 2 +// break +// } +// case 1 { +// x := 3 +// z := 3 +// continue +// } +// case 2 { +// x := 4 +// break +// } +// case 3 { +// x := 5 +// z := 5 +// continue +// } +// mstore(y, 9) +// } +// mstore(x, 0x42) +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_nested.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_nested.yul new file mode 100644 index 000000000..f2393f817 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_nested.yul @@ -0,0 +1,76 @@ +{ + let x := 1 + let y := 1 + let a := 7 + let b := 9 + for { } calldataload(0) { } + { + y := 9 + mstore(a, 7) + if callvalue() { + x := 2 + for {} calldataload(1) {} + { + a := 2 // can be removed + if eq(x, 3) { + b := 3 // cannot be removed + y := 2 // will be removed + continue + } + } + mstore(b, 2) + break + } + if eq(callvalue(), 3) { + x := 12 + y := 12 + continue + } + x := 3 + mstore(y, 3) + } + mstore(x, 0x42) +} +// ---- +// redundantAssignEliminator +// { +// let x := 1 +// let y := 1 +// let a := 7 +// let b := 9 +// for { +// } +// calldataload(0) +// { +// } +// { +// y := 9 +// mstore(a, 7) +// if callvalue() +// { +// x := 2 +// for { +// } +// calldataload(1) +// { +// } +// { +// if eq(x, 3) +// { +// b := 3 +// continue +// } +// } +// mstore(b, 2) +// break +// } +// if eq(callvalue(), 3) +// { +// x := 12 +// continue +// } +// x := 3 +// mstore(y, 3) +// } +// mstore(x, 0x42) +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_stmnts_after_break_continue.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_stmnts_after_break_continue.yul new file mode 100644 index 000000000..a3cec65c1 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_stmnts_after_break_continue.yul @@ -0,0 +1,52 @@ +{ + let x := 1 + let y := 1 + for { } calldataload(0) { } + { + y := 9 + if callvalue() { + x := 2 + y := 2 // will be removed + break + x := 7 // after break, we start with fresh state. + } + if eq(callvalue(), 3) { + x := 12 + y := 12 // will be removed + continue + x := 17 // after continue, we start with fresh state. + y := 9 + } + x := 3 + mstore(y, 3) + } + mstore(x, 0x42) +} +// ---- +// redundantAssignEliminator +// { +// let x := 1 +// let y := 1 +// for { +// } +// calldataload(0) +// { +// } +// { +// y := 9 +// if callvalue() +// { +// x := 2 +// break +// } +// if eq(callvalue(), 3) +// { +// x := 12 +// continue +// y := 9 +// } +// x := 3 +// mstore(y, 3) +// } +// mstore(x, 0x42) +// }