diff --git a/libyul/optimiser/RedundantAssignEliminator.cpp b/libyul/optimiser/RedundantAssignEliminator.cpp index 48f0e7fba..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. @@ -136,6 +135,8 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) TrackedAssignments zeroRuns{m_assignments}; (*this)(_forLoop.body); + merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts)); + m_forLoopInfo.pendingContinueStmts = {}; (*this)(_forLoop.post); visit(*_forLoop.condition); @@ -143,6 +144,9 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) TrackedAssignments oneRun{m_assignments}; (*this)(_forLoop.body); + + merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts)); + m_forLoopInfo.pendingContinueStmts.clear(); (*this)(_forLoop.post); visit(*_forLoop.condition); @@ -150,25 +154,40 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop) // Order does not matter because "max" is commutative and associative. merge(m_assignments, move(oneRun)); merge(m_assignments, move(zeroRuns)); + merge(m_assignments, move(m_forLoopInfo.pendingBreakStmts)); + m_forLoopInfo.pendingBreakStmts.clear(); + + 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&) { - yulAssert(false, "Not implemented yet."); + m_forLoopInfo.pendingBreakStmts.emplace_back(move(m_assignments)); + m_assignments.clear(); } void RedundantAssignEliminator::operator()(Continue const&) { - yulAssert(false, "Not implemented yet."); + 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) @@ -218,24 +237,45 @@ void RedundantAssignEliminator::merge(TrackedAssignments& _target, TrackedAssign }); } +void RedundantAssignEliminator::merge(TrackedAssignments& _target, vector&& _source) +{ + for (TrackedAssignments& ts: _source) + merge(_target, move(ts)); + _source.clear(); +} + void RedundantAssignEliminator::changeUndecidedTo(YulString _variable, RedundantAssignEliminator::State _newState) { for (auto& assignment: m_assignments[_variable]) - if (assignment.second == State{State::Undecided}) + if (assignment.second == State::Undecided) assignment.second = _newState; } -void RedundantAssignEliminator::finalize(YulString _variable) +void RedundantAssignEliminator::finalize(YulString _variable, RedundantAssignEliminator::State _finalState) { - for (auto& assignment: m_assignments[_variable]) + finalize(m_assignments, _variable, _finalState); + for (auto& assignments: m_forLoopInfo.pendingBreakStmts) + finalize(assignments, _variable, _finalState); + for (auto& assignments: m_forLoopInfo.pendingContinueStmts) + finalize(assignments, _variable, _finalState); +} + +void RedundantAssignEliminator::finalize( + TrackedAssignments& _assignments, + YulString _variable, + RedundantAssignEliminator::State _finalState +) +{ + for (auto const& assignment: _assignments[_variable]) { - assertThrow(assignment.second != State::Undecided, OptimizerException, ""); - if (assignment.second == State{State::Unused} && MovableChecker{*m_dialect, *assignment.first->value}.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 8ccd37fae..3165d0335 100644 --- a/libyul/optimiser/RedundantAssignEliminator.h +++ b/libyul/optimiser/RedundantAssignEliminator.h @@ -25,6 +25,7 @@ #include #include +#include namespace yul { @@ -136,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>; @@ -171,13 +145,29 @@ private: /// above. /// Will destroy @a _source. 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; std::set m_pendingRemovals; TrackedAssignments m_assignments; + + /// Working data for traversing for-loops. + struct ForLoopInfo + { + /// Tracked assignment states for each break statement. + std::vector pendingBreakStmts; + /// Tracked assignment states for each continue statement. + std::vector pendingContinueStmts; + }; + ForLoopInfo m_forLoopInfo; }; class AssignmentRemover: public ASTModifier diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul new file mode 100644 index 000000000..8919a0309 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_break.yul @@ -0,0 +1,34 @@ +{ + 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 +// x := 1 +// for { +// } +// calldataload(0) +// { +// } +// { +// if callvalue() +// { +// x := 2 +// break +// } +// x := 3 +// } +// mstore(x, 0x42) +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul new file mode 100644 index 000000000..e21b76a77 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue.yul @@ -0,0 +1,35 @@ +{ + 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 +// for { +// } +// calldataload(0) +// { +// } +// { +// x := 2 +// if callvalue() +// { +// continue +// } +// mstore(x, 2) +// } +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul new file mode 100644 index 000000000..ef1cac231 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_2.yul @@ -0,0 +1,34 @@ +{ + 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 +// x := 1 +// for { +// } +// calldataload(0) +// { +// } +// { +// if callvalue() +// { +// x := 2 +// continue +// } +// x := 3 +// } +// mstore(x, 0x42) +// } diff --git a/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul new file mode 100644 index 000000000..246d97248 --- /dev/null +++ b/test/libyul/yulOptimizerTests/redundantAssignEliminator/for_continue_3.yul @@ -0,0 +1,32 @@ +{ + 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 +// for { +// } +// calldataload(0) +// { +// mstore(x, 0x42) +// } +// { +// if callvalue() +// { +// x := 2 +// continue +// } +// x := 3 +// } +// } 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) +// }