From ec8cd56c4fb13f081d92800fa9f74abe26cc4270 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 11:49:40 +0100 Subject: [PATCH 01/26] Re-implement KnowledgeBase using groups of constantly-spaced variables. --- libyul/optimiser/DataFlowAnalyzer.cpp | 4 +- libyul/optimiser/KnowledgeBase.cpp | 144 +++++++++++++++------ libyul/optimiser/KnowledgeBase.h | 57 ++++++-- libyul/optimiser/UnusedStoreEliminator.cpp | 8 +- test/libyul/KnowledgeBaseTest.cpp | 8 +- 5 files changed, 165 insertions(+), 56 deletions(-) diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index 1d5519b5781a..11532a885e48 100644 --- a/libyul/optimiser/DataFlowAnalyzer.cpp +++ b/libyul/optimiser/DataFlowAnalyzer.cpp @@ -49,7 +49,7 @@ DataFlowAnalyzer::DataFlowAnalyzer( ): m_dialect(_dialect), m_functionSideEffects(std::move(_functionSideEffects)), - m_knowledgeBase(_dialect, [this](YulString _var) { return variableValue(_var); }), + m_knowledgeBase([this](YulString _var) { return variableValue(_var); }), m_analyzeStores(_analyzeStores == MemoryAndStorage::Analyze) { if (m_analyzeStores) @@ -75,7 +75,7 @@ void DataFlowAnalyzer::operator()(ExpressionStatement& _statement) cxx20::erase_if(m_state.environment.storage, mapTuple([&](auto&& key, auto&& value) { return !m_knowledgeBase.knownToBeDifferent(vars->first, key) && - !m_knowledgeBase.knownToBeEqual(vars->second, value); + vars->second != value; })); m_state.environment.storage[vars->first] = vars->second; return; diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 77848cce0475..2fd5fb0a9bab 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -23,7 +23,6 @@ #include #include -#include #include #include @@ -36,37 +35,24 @@ using namespace solidity::yul; bool KnowledgeBase::knownToBeDifferent(YulString _a, YulString _b) { - // Try to use the simplification rules together with the - // current values to turn `sub(_a, _b)` into a nonzero constant. - // If that fails, try `eq(_a, _b)`. - if (optional difference = differenceIfKnownConstant(_a, _b)) return difference != 0; - - Expression expr2 = simplify(FunctionCall{{}, {{}, "eq"_yulstring}, util::make_vector(Identifier{{}, _a}, Identifier{{}, _b})}); - if (holds_alternative(expr2)) - return valueOfLiteral(std::get(expr2)) == 0; - return false; } optional KnowledgeBase::differenceIfKnownConstant(YulString _a, YulString _b) { - // Try to use the simplification rules together with the - // current values to turn `sub(_a, _b)` into a constant. - - Expression expr1 = simplify(FunctionCall{{}, {{}, "sub"_yulstring}, util::make_vector(Identifier{{}, _a}, Identifier{{}, _b})}); - if (Literal const* value = get_if(&expr1)) - return valueOfLiteral(*value); - - return {}; + VariableOffset offA = explore(_a); + VariableOffset offB = explore(_b); + if (offA.reference == offB.reference) + return offA.offset - offB.offset; + else + return {}; } + bool KnowledgeBase::knownToBeDifferentByAtLeast32(YulString _a, YulString _b) { - // Try to use the simplification rules together with the - // current values to turn `sub(_a, _b)` into a constant whose absolute value is at least 32. - if (optional difference = differenceIfKnownConstant(_a, _b)) return difference >= 32 && difference <= u256(0) - 32; @@ -80,29 +66,113 @@ bool KnowledgeBase::knownToBeZero(YulString _a) optional KnowledgeBase::valueIfKnownConstant(YulString _a) { - if (AssignedValue const* value = m_variableValues(_a)) - if (Literal const* literal = get_if(value->value)) - return valueOfLiteral(*literal); - return {}; + VariableOffset offset = explore(_a); + if (offset.reference == YulString{}) + return offset.offset; + else + return nullopt; +} + +optional KnowledgeBase::valueIfKnownConstant(Expression const& _expression) +{ + if (Identifier const* ident = get_if(&_expression)) + return valueIfKnownConstant(ident->name); + else if (Literal const* lit = get_if(&_expression)) + return valueOfLiteral(*lit); + else + return {}; } -Expression KnowledgeBase::simplify(Expression _expression) +KnowledgeBase::VariableOffset KnowledgeBase::explore(YulString _var) { - m_counter = 0; - return simplifyRecursively(std::move(_expression)); + // We query the value first so that the variable is reset if it has changed + // since the last call. + Expression const* value = valueOf(_var); + if (VariableOffset const* varOff = util::valueOrNullptr(m_offsets, _var)) + return *varOff; + + if (value) + if (optional offset = explore(*value)) + return setOffset(_var, *offset); + return setOffset(_var, VariableOffset{_var, 0}); + } -Expression KnowledgeBase::simplifyRecursively(Expression _expression) +optional KnowledgeBase::explore(Expression const& _value) { - if (m_counter++ > 100) - return _expression; + if (Literal const* literal = std::get_if(&_value)) + return VariableOffset{YulString{}, valueOfLiteral(*literal)}; + else if (Identifier const* identifier = std::get_if(&_value)) + return explore(identifier->name); + else if (FunctionCall const* f = get_if(&_value)) + if (f->functionName.name == "add"_yulstring || f->functionName.name == "sub"_yulstring) + if (optional a = explore(f->arguments[0])) + if (optional b = explore(f->arguments[1])) + { + u256 offset = + f->functionName.name == "add"_yulstring ? + a->offset + b->offset : + a->offset - b->offset; + if (a->reference == b->reference) + // Offsets relative to the same reference variable + return VariableOffset{a->reference, offset}; + else if (a->reference == YulString{}) + // a is constant + return VariableOffset{b->reference, offset}; + else if (b->reference == YulString{}) + // b is constant + return VariableOffset{a->reference, offset}; + } - if (holds_alternative(_expression)) - for (Expression& arg: std::get(_expression).arguments) - arg = simplifyRecursively(arg); + return {}; +} - if (auto match = SimplificationRules::findFirstMatch(_expression, m_dialect, m_variableValues)) - return simplifyRecursively(match->action().toExpression(debugDataOf(_expression))); +Expression const* KnowledgeBase::valueOf(YulString _var) +{ + Expression const* lastValue = m_lastKnownValue[_var]; + AssignedValue const* assignedValue = m_variableValues(_var); + Expression const* currentValue = assignedValue ? assignedValue->value : nullptr; + if (lastValue != currentValue) + reset(_var); + m_lastKnownValue[_var] = currentValue; + return currentValue; +} - return _expression; +void KnowledgeBase::reset(YulString _var) +{ + m_lastKnownValue.erase(_var); + if (VariableOffset const* offset = util::valueOrNullptr(m_offsets, _var)) + { + // Remove var from its group + if (offset->reference != YulString{}) + m_groupMembers[offset->reference].erase(_var); + m_offsets.erase(_var); + } + if (set* group = util::valueOrNullptr(m_groupMembers, _var)) + { + // _var was a representative, we might have to find a new one. + if (group->empty()) + m_groupMembers.erase(_var); + else + { + YulString newRepresentative = *group->begin(); + u256 newOffset = m_offsets[newRepresentative].offset; + for (YulString groupMember: *group) + { + yulAssert(m_offsets[groupMember].reference == _var); + m_offsets[groupMember].reference = newRepresentative; + m_offsets[newRepresentative].offset -= newOffset; + } + } + } +} + +KnowledgeBase::VariableOffset KnowledgeBase::setOffset(YulString _variable, VariableOffset _value) +{ + m_offsets[_variable] = _value; + // Constants are not tracked in m_groupMembers because + // the "representative" can never be reset. + if (_value.reference != YulString{}) + m_groupMembers[_value.reference].insert(_variable); + return _value; } diff --git a/libyul/optimiser/KnowledgeBase.h b/libyul/optimiser/KnowledgeBase.h index 999d0e312bc4..82c82a7e93ee 100644 --- a/libyul/optimiser/KnowledgeBase.h +++ b/libyul/optimiser/KnowledgeBase.h @@ -38,32 +38,69 @@ struct AssignedValue; /** * Class that can answer questions about values of variables and their relations. + * + * Requires a callback that returns the current value of the variable. + * The value can change any time during the lifetime of the KnowledgeBase, + * it will update its internal data structure accordingly. + * + * This means that the code the KnowledgeBase is used on does not need to be in SSA + * form. + * The only requirement is that the assigned values are movable expressions. + * + * Internally, tries to find groups of variables that have a mutual constant + * difference and stores these differences always relative to a specific + * representative variable of the group. + * + * There is a special group which is the constant values. Those use the + * empty YulString as representative "variable". */ class KnowledgeBase { public: - KnowledgeBase( - Dialect const& _dialect, - std::function _variableValues - ): - m_dialect(_dialect), + KnowledgeBase(std::function _variableValues): m_variableValues(std::move(_variableValues)) {} bool knownToBeDifferent(YulString _a, YulString _b); std::optional differenceIfKnownConstant(YulString _a, YulString _b); bool knownToBeDifferentByAtLeast32(YulString _a, YulString _b); - bool knownToBeEqual(YulString _a, YulString _b) const { return _a == _b; } bool knownToBeZero(YulString _a); std::optional valueIfKnownConstant(YulString _a); + std::optional valueIfKnownConstant(Expression const& _expression); private: - Expression simplify(Expression _expression); - Expression simplifyRecursively(Expression _expression); + /** + * Constant offset relative to a reference variable, or absolute constant if the + * reference variable is the empty YulString. + */ + struct VariableOffset + { + YulString reference; + u256 offset; + }; - Dialect const& m_dialect; + VariableOffset explore(YulString _var); + std::optional explore(Expression const& _value); + + /// Retrieves the current value of a variable and potentially resets the variable if it is not up to date. + Expression const* valueOf(YulString _var); + + /// Resets all information about the variable and removes it from its group, + /// potentially finding a new representative. + void reset(YulString _var); + + VariableOffset setOffset(YulString _variable, VariableOffset _value); + + /// Callback to retrieve the current value of a variable. std::function m_variableValues; - size_t m_counter = 0; + + /// Offsets for each variable to one representative per group. + /// The empty string is the representative of the constant value zero. + std::map m_offsets; + /// Last known value of each variable we queried. + std::map m_lastKnownValue; + /// For each representative, variables that use it to offset from. + std::map> m_groupMembers; }; } diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 2e96be2e703a..d5155ad7e55b 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -174,7 +174,7 @@ void UnusedStoreEliminator::visit(Statement const& _statement) initialState = State::Used; auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); auto length = identifierNameIfSSA(funCall->arguments.at(2)); - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + KnowledgeBase knowledge([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); if (length && startOffset) { FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); @@ -267,7 +267,7 @@ bool UnusedStoreEliminator::knownUnrelated( UnusedStoreEliminator::Operation const& _op2 ) const { - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + KnowledgeBase knowledge([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); if (_op1.location != _op2.location) return true; @@ -348,7 +348,7 @@ bool UnusedStoreEliminator::knownCovered( return true; if (_covered.location == Location::Memory) { - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + KnowledgeBase knowledge([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); if (_covered.length && knowledge.knownToBeZero(*_covered.length)) return true; @@ -359,7 +359,7 @@ bool UnusedStoreEliminator::knownCovered( return false; optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); - if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) + if (*_covered.start == *_covering.start) if (coveredLength && coveringLength && *coveredLength <= *coveringLength) return true; optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); diff --git a/test/libyul/KnowledgeBaseTest.cpp b/test/libyul/KnowledgeBaseTest.cpp index ec2f0313d18e..7cb5a8ae3c05 100644 --- a/test/libyul/KnowledgeBaseTest.cpp +++ b/test/libyul/KnowledgeBaseTest.cpp @@ -58,7 +58,7 @@ class KnowledgeBaseTest for (auto const& [name, expression]: m_ssaValues.values()) m_values[name].value = expression; - return KnowledgeBase(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_values, _var); }); + return KnowledgeBase([this](YulString _var) { return util::valueOrNullptr(m_values, _var); }); } EVMDialect m_dialect{EVMVersion{}, true}; @@ -83,9 +83,11 @@ BOOST_AUTO_TEST_CASE(basic) BOOST_CHECK(!kb.knownToBeDifferent("a"_yulstring, "b"_yulstring)); // This only works if the variable names are the same. // It assumes that SSA+CSE+Simplifier actually replaces the variables. - BOOST_CHECK(!kb.knownToBeEqual("a"_yulstring, "b"_yulstring)); BOOST_CHECK(!kb.valueIfKnownConstant("a"_yulstring)); BOOST_CHECK(kb.valueIfKnownConstant("zero"_yulstring) == u256(0)); + BOOST_CHECK(kb.differenceIfKnownConstant("a"_yulstring, "b"_yulstring) == u256(0)); + BOOST_CHECK(kb.differenceIfKnownConstant("a"_yulstring, "c"_yulstring) == u256(0)); + BOOST_CHECK(kb.valueIfKnownConstant("e"_yulstring) == u256(0)); } BOOST_AUTO_TEST_CASE(difference) @@ -94,7 +96,7 @@ BOOST_AUTO_TEST_CASE(difference) let a := calldataload(0) let b := add(a, 200) let c := add(a, 220) - let d := add(c, 12) + let d := add(12, c) let e := sub(c, 12) })"); From 5efe31cd7c18608bcfa632a136a9b8bad51a833f Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 14:06:37 +0100 Subject: [PATCH 02/26] Keep one instance of KnowledgeBase for UnusedStoreEliminator. --- libyul/optimiser/UnusedStoreEliminator.cpp | 60 +++++++++++++--------- libyul/optimiser/UnusedStoreEliminator.h | 11 ++-- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index d5155ad7e55b..24553339526b 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -92,6 +92,21 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) remover(_ast); } +UnusedStoreEliminator::UnusedStoreEliminator( + Dialect const& _dialect, + map const& _functionSideEffects, + map _controlFlowSideEffects, + map const& _ssaValues, + bool _ignoreMemory +): + UnusedStoreBase(_dialect), + m_ignoreMemory(_ignoreMemory), + m_functionSideEffects(_functionSideEffects), + m_controlFlowSideEffects(_controlFlowSideEffects), + m_ssaValues(_ssaValues), + m_knowledgeBase([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }) +{} + void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) { UnusedStoreBase::operator()(_functionCall); @@ -174,12 +189,11 @@ void UnusedStoreEliminator::visit(Statement const& _statement) initialState = State::Used; auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); auto length = identifierNameIfSSA(funCall->arguments.at(2)); - KnowledgeBase knowledge([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); if (length && startOffset) { FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); if ( - knowledge.knownToBeZero(*startOffset) && + m_knowledgeBase.knownToBeZero(*startOffset) && lengthCall && toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE ) @@ -267,8 +281,6 @@ bool UnusedStoreEliminator::knownUnrelated( UnusedStoreEliminator::Operation const& _op2 ) const { - KnowledgeBase knowledge([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - if (_op1.location != _op2.location) return true; if (_op1.location == Location::Storage) @@ -278,26 +290,26 @@ bool UnusedStoreEliminator::knownUnrelated( yulAssert( _op1.length && _op2.length && - knowledge.valueIfKnownConstant(*_op1.length) == 1 && - knowledge.valueIfKnownConstant(*_op2.length) == 1 + m_knowledgeBase.valueIfKnownConstant(*_op1.length) == 1 && + m_knowledgeBase.valueIfKnownConstant(*_op2.length) == 1 ); - return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); + return m_knowledgeBase.knownToBeDifferent(*_op1.start, *_op2.start); } } else { yulAssert(_op1.location == Location::Memory, ""); if ( - (_op1.length && knowledge.knownToBeZero(*_op1.length)) || - (_op2.length && knowledge.knownToBeZero(*_op2.length)) + (_op1.length && m_knowledgeBase.knownToBeZero(*_op1.length)) || + (_op2.length && m_knowledgeBase.knownToBeZero(*_op2.length)) ) return true; if (_op1.start && _op1.length && _op2.start) { - optional length1 = knowledge.valueIfKnownConstant(*_op1.length); - optional start1 = knowledge.valueIfKnownConstant(*_op1.start); - optional start2 = knowledge.valueIfKnownConstant(*_op2.start); + optional length1 = m_knowledgeBase.valueIfKnownConstant(*_op1.length); + optional start1 = m_knowledgeBase.valueIfKnownConstant(*_op1.start); + optional start2 = m_knowledgeBase.valueIfKnownConstant(*_op2.start); if ( (length1 && start1 && start2) && *start1 + *length1 >= *start1 && // no overflow @@ -307,9 +319,9 @@ bool UnusedStoreEliminator::knownUnrelated( } if (_op2.start && _op2.length && _op1.start) { - optional length2 = knowledge.valueIfKnownConstant(*_op2.length); - optional start2 = knowledge.valueIfKnownConstant(*_op2.start); - optional start1 = knowledge.valueIfKnownConstant(*_op1.start); + optional length2 = m_knowledgeBase.valueIfKnownConstant(*_op2.length); + optional start2 = m_knowledgeBase.valueIfKnownConstant(*_op2.start); + optional start1 = m_knowledgeBase.valueIfKnownConstant(*_op1.start); if ( (length2 && start2 && start1) && *start2 + *length2 >= *start2 && // no overflow @@ -320,12 +332,12 @@ bool UnusedStoreEliminator::knownUnrelated( if (_op1.start && _op1.length && _op2.start && _op2.length) { - optional length1 = knowledge.valueIfKnownConstant(*_op1.length); - optional length2 = knowledge.valueIfKnownConstant(*_op2.length); + optional length1 = m_knowledgeBase.valueIfKnownConstant(*_op1.length); + optional length2 = m_knowledgeBase.valueIfKnownConstant(*_op2.length); if ( (length1 && *length1 <= 32) && (length2 && *length2 <= 32) && - knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) + m_knowledgeBase.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) ) return true; } @@ -348,22 +360,20 @@ bool UnusedStoreEliminator::knownCovered( return true; if (_covered.location == Location::Memory) { - KnowledgeBase knowledge([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - - if (_covered.length && knowledge.knownToBeZero(*_covered.length)) + if (_covered.length && m_knowledgeBase.knownToBeZero(*_covered.length)) return true; // Condition (i = cover_i_ng, e = cover_e_d): // i.start <= e.start && e.start + e.length <= i.start + i.length if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) return false; - optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); - optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); + optional coveredLength = m_knowledgeBase.valueIfKnownConstant(*_covered.length); + optional coveringLength = m_knowledgeBase.valueIfKnownConstant(*_covering.length); if (*_covered.start == *_covering.start) if (coveredLength && coveringLength && *coveredLength <= *coveringLength) return true; - optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); - optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); + optional coveredStart = m_knowledgeBase.valueIfKnownConstant(*_covered.start); + optional coveringStart = m_knowledgeBase.valueIfKnownConstant(*_covering.start); if (coveredStart && coveringStart && coveredLength && coveringLength) if ( *coveringStart <= *coveredStart && diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index 8b5bfd7b140b..7fbf9885ff3c 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -68,13 +69,7 @@ class UnusedStoreEliminator: public UnusedStoreBase std::map _controlFlowSideEffects, std::map const& _ssaValues, bool _ignoreMemory - ): - UnusedStoreBase(_dialect), - m_ignoreMemory(_ignoreMemory), - m_functionSideEffects(_functionSideEffects), - m_controlFlowSideEffects(_controlFlowSideEffects), - m_ssaValues(_ssaValues) - {} + ); using UnusedStoreBase::operator(); void operator()(FunctionCall const& _functionCall) override; @@ -121,6 +116,8 @@ class UnusedStoreEliminator: public UnusedStoreBase std::map const& m_ssaValues; std::map m_storeOperations; + + KnowledgeBase mutable m_knowledgeBase; }; } From 94fad23bd0ffd82ae82327b0dbab75d205292739 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 14:22:40 +0100 Subject: [PATCH 03/26] Optimize in case this is SSA. --- libyul/optimiser/KnowledgeBase.cpp | 34 ++++++++++++++++++---- libyul/optimiser/KnowledgeBase.h | 9 ++++++ libyul/optimiser/UnusedStoreEliminator.cpp | 2 +- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 2fd5fb0a9bab..23627653a5b1 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -33,6 +33,11 @@ using namespace std; using namespace solidity; using namespace solidity::yul; +KnowledgeBase::KnowledgeBase(map const& _ssaValues): + m_valuesAreSSA(true), + m_variableValues([_ssaValues](YulString _var) { return util::valueOrNullptr(_ssaValues, _var); }) +{} + bool KnowledgeBase::knownToBeDifferent(YulString _a, YulString _b) { if (optional difference = differenceIfKnownConstant(_a, _b)) @@ -85,11 +90,23 @@ optional KnowledgeBase::valueIfKnownConstant(Expression const& _expression KnowledgeBase::VariableOffset KnowledgeBase::explore(YulString _var) { - // We query the value first so that the variable is reset if it has changed - // since the last call. - Expression const* value = valueOf(_var); - if (VariableOffset const* varOff = util::valueOrNullptr(m_offsets, _var)) - return *varOff; + Expression const* value = nullptr; + if (m_valuesAreSSA) + { + // In SSA, a once determined offset is always valid, so we first see + // if we already computed it. + if (VariableOffset const* varOff = util::valueOrNullptr(m_offsets, _var)) + return *varOff; + value = valueOf(_var); + } + else + { + // For non-SSA, we query the value first so that the variable is reset if it has changed + // since the last call. + value = valueOf(_var); + if (VariableOffset const* varOff = util::valueOrNullptr(m_offsets, _var)) + return *varOff; + } if (value) if (optional offset = explore(*value)) @@ -129,9 +146,12 @@ optional KnowledgeBase::explore(Expression const& Expression const* KnowledgeBase::valueOf(YulString _var) { - Expression const* lastValue = m_lastKnownValue[_var]; AssignedValue const* assignedValue = m_variableValues(_var); Expression const* currentValue = assignedValue ? assignedValue->value : nullptr; + if (m_valuesAreSSA) + return currentValue; + + Expression const* lastValue = m_lastKnownValue[_var]; if (lastValue != currentValue) reset(_var); m_lastKnownValue[_var] = currentValue; @@ -140,6 +160,8 @@ Expression const* KnowledgeBase::valueOf(YulString _var) void KnowledgeBase::reset(YulString _var) { + yulAssert(!m_valuesAreSSA); + m_lastKnownValue.erase(_var); if (VariableOffset const* offset = util::valueOrNullptr(m_offsets, _var)) { diff --git a/libyul/optimiser/KnowledgeBase.h b/libyul/optimiser/KnowledgeBase.h index 82c82a7e93ee..934b2e21ad94 100644 --- a/libyul/optimiser/KnowledgeBase.h +++ b/libyul/optimiser/KnowledgeBase.h @@ -47,6 +47,9 @@ struct AssignedValue; * form. * The only requirement is that the assigned values are movable expressions. * + * There is a constructor to provide all SSA values right at the beginning. + * If you use this, the KnowledgeBase will be slightly more efficient. + * * Internally, tries to find groups of variables that have a mutual constant * difference and stores these differences always relative to a specific * representative variable of the group. @@ -57,9 +60,13 @@ struct AssignedValue; class KnowledgeBase { public: + /// Constructor for arbitrary value callback that allows for variable values + /// to change in between calls to functions of this class. KnowledgeBase(std::function _variableValues): m_variableValues(std::move(_variableValues)) {} + /// Constructor to use if source code is in SSA form and values are constant. + KnowledgeBase(std::map const& _ssaValues); bool knownToBeDifferent(YulString _a, YulString _b); std::optional differenceIfKnownConstant(YulString _a, YulString _b); @@ -91,6 +98,8 @@ class KnowledgeBase VariableOffset setOffset(YulString _variable, VariableOffset _value); + /// If true, we can assume that variable values never change and skip some steps. + bool m_valuesAreSSA = false; /// Callback to retrieve the current value of a variable. std::function m_variableValues; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 24553339526b..3df9c6319638 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -104,7 +104,7 @@ UnusedStoreEliminator::UnusedStoreEliminator( m_functionSideEffects(_functionSideEffects), m_controlFlowSideEffects(_controlFlowSideEffects), m_ssaValues(_ssaValues), - m_knowledgeBase([this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }) + m_knowledgeBase(_ssaValues) {} void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) From 2d028e35072728ec2ec53c8bc48522552361c068 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 16:04:19 +0100 Subject: [PATCH 04/26] Bugfix. --- libyul/optimiser/KnowledgeBase.cpp | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 23627653a5b1..558dc8988800 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -122,24 +122,33 @@ optional KnowledgeBase::explore(Expression const& else if (Identifier const* identifier = std::get_if(&_value)) return explore(identifier->name); else if (FunctionCall const* f = get_if(&_value)) - if (f->functionName.name == "add"_yulstring || f->functionName.name == "sub"_yulstring) + { + if (f->functionName.name == "add"_yulstring) + { if (optional a = explore(f->arguments[0])) if (optional b = explore(f->arguments[1])) { - u256 offset = - f->functionName.name == "add"_yulstring ? - a->offset + b->offset : - a->offset - b->offset; - if (a->reference == b->reference) - // Offsets relative to the same reference variable - return VariableOffset{a->reference, offset}; - else if (a->reference == YulString{}) + u256 offset = a->offset + b->offset; + if (a->reference.empty()) // a is constant return VariableOffset{b->reference, offset}; - else if (b->reference == YulString{}) + else if (b->reference.empty()) // b is constant return VariableOffset{a->reference, offset}; } + } + else if (f->functionName.name == "sub"_yulstring) + if (optional a = explore(f->arguments[0])) + if (optional b = explore(f->arguments[1])) + { + u256 offset = a->offset - b->offset; + if (a->reference == b->reference) + return VariableOffset{YulString{}, offset}; + else if (b->reference.empty()) + // b is constant + return VariableOffset{a->reference, offset}; + } + } return {}; } From f70462cdba1108e185173d032512ae46f29b39d3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 17:14:01 +0100 Subject: [PATCH 05/26] Update gas costs. --- .../array/copying/function_type_array_to_storage.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/libsolidity/semanticTests/array/copying/function_type_array_to_storage.sol b/test/libsolidity/semanticTests/array/copying/function_type_array_to_storage.sol index 1e8760250a37..409bb2983cc8 100644 --- a/test/libsolidity/semanticTests/array/copying/function_type_array_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/function_type_array_to_storage.sol @@ -46,7 +46,7 @@ contract C { } // ---- // test() -> 0x20, 0x14, "[a called][b called]" -// gas irOptimized: 116673 +// gas irOptimized: 116660 // gas legacy: 119030 // gas legacyOptimized: 117021 // test2() -> 0x20, 0x14, "[b called][a called]" From 53a44bd5aedbbd252a3e7992480243961ffe7ae9 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 18:02:56 +0100 Subject: [PATCH 06/26] Review changes. --- libyul/optimiser/KnowledgeBase.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 558dc8988800..00d7cda1c7ef 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -66,13 +66,13 @@ bool KnowledgeBase::knownToBeDifferentByAtLeast32(YulString _a, YulString _b) bool KnowledgeBase::knownToBeZero(YulString _a) { - return valueIfKnownConstant(_a) == u256{}; + return valueIfKnownConstant(_a) == 0; } optional KnowledgeBase::valueIfKnownConstant(YulString _a) { VariableOffset offset = explore(_a); - if (offset.reference == YulString{}) + if (offset.reference.empty()) return offset.offset; else return nullopt; @@ -85,7 +85,7 @@ optional KnowledgeBase::valueIfKnownConstant(Expression const& _expression else if (Literal const* lit = get_if(&_expression)) return valueOfLiteral(*lit); else - return {}; + return nullopt; } KnowledgeBase::VariableOffset KnowledgeBase::explore(YulString _var) @@ -150,7 +150,7 @@ optional KnowledgeBase::explore(Expression const& } } - return {}; + return nullopt; } Expression const* KnowledgeBase::valueOf(YulString _var) @@ -175,7 +175,7 @@ void KnowledgeBase::reset(YulString _var) if (VariableOffset const* offset = util::valueOrNullptr(m_offsets, _var)) { // Remove var from its group - if (offset->reference != YulString{}) + if (!offset->reference.empty()) m_groupMembers[offset->reference].erase(_var); m_offsets.erase(_var); } @@ -203,7 +203,7 @@ KnowledgeBase::VariableOffset KnowledgeBase::setOffset(YulString _variable, Vari m_offsets[_variable] = _value; // Constants are not tracked in m_groupMembers because // the "representative" can never be reset. - if (_value.reference != YulString{}) + if (!_value.reference.empty()) m_groupMembers[_value.reference].insert(_variable); return _value; } From 5c85818f509c917c1d7a6a6e1dcb47b54060545f Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 30 Nov 2022 10:06:44 +0100 Subject: [PATCH 07/26] Update libyul/optimiser/KnowledgeBase.cpp Co-authored-by: matheusaaguiar <95899911+matheusaaguiar@users.noreply.github.com> --- libyul/optimiser/KnowledgeBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 00d7cda1c7ef..36f07b74aa2c 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -52,7 +52,7 @@ optional KnowledgeBase::differenceIfKnownConstant(YulString _a, YulString if (offA.reference == offB.reference) return offA.offset - offB.offset; else - return {}; + return nullopt; } From f5f3eaacb9b8a82df99c3f4423b875a5f71c8006 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 30 Nov 2022 10:07:31 +0100 Subject: [PATCH 08/26] Apply suggestions from code review Co-authored-by: matheusaaguiar <95899911+matheusaaguiar@users.noreply.github.com> --- libyul/optimiser/KnowledgeBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 36f07b74aa2c..4b90acf9fdca 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -117,9 +117,9 @@ KnowledgeBase::VariableOffset KnowledgeBase::explore(YulString _var) optional KnowledgeBase::explore(Expression const& _value) { - if (Literal const* literal = std::get_if(&_value)) + if (Literal const* literal = get_if(&_value)) return VariableOffset{YulString{}, valueOfLiteral(*literal)}; - else if (Identifier const* identifier = std::get_if(&_value)) + else if (Identifier const* identifier = get_if(&_value)) return explore(identifier->name); else if (FunctionCall const* f = get_if(&_value)) { From 135f9654885aad08513250b8183855811c7cd059 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 30 Nov 2022 10:21:02 +0100 Subject: [PATCH 09/26] Review fixes. --- libyul/optimiser/KnowledgeBase.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/libyul/optimiser/KnowledgeBase.cpp b/libyul/optimiser/KnowledgeBase.cpp index 4b90acf9fdca..96b352e402d9 100644 --- a/libyul/optimiser/KnowledgeBase.cpp +++ b/libyul/optimiser/KnowledgeBase.cpp @@ -182,19 +182,26 @@ void KnowledgeBase::reset(YulString _var) if (set* group = util::valueOrNullptr(m_groupMembers, _var)) { // _var was a representative, we might have to find a new one. - if (group->empty()) - m_groupMembers.erase(_var); - else + if (!group->empty()) { YulString newRepresentative = *group->begin(); + yulAssert(newRepresentative != _var); u256 newOffset = m_offsets[newRepresentative].offset; + // newOffset = newRepresentative - _var for (YulString groupMember: *group) { yulAssert(m_offsets[groupMember].reference == _var); m_offsets[groupMember].reference = newRepresentative; - m_offsets[newRepresentative].offset -= newOffset; + // groupMember = _var + m_offsets[groupMember].offset (old) + // = newRepresentative - newOffset + m_offsets[groupMember].offset (old) + // so subtracting newOffset from .offset yields the original relation again, + // just with _var replaced by newRepresentative + m_offsets[groupMember].offset -= newOffset; } + m_groupMembers[newRepresentative] = std::move(*group); + } + m_groupMembers.erase(_var); } } From bd7676873e34957d4d865cd92cd354c0fbab3ad8 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 23 Nov 2022 15:28:11 +0100 Subject: [PATCH 10/26] modify unused store --- libyul/optimiser/UnusedAssignEliminator.cpp | 95 +-- libyul/optimiser/UnusedAssignEliminator.h | 9 +- libyul/optimiser/UnusedStoreBase.cpp | 52 +- libyul/optimiser/UnusedStoreBase.h | 36 +- libyul/optimiser/UnusedStoreEliminator.cpp | 706 +++++++++--------- libyul/optimiser/UnusedStoreEliminator.h | 88 +-- .../for_deep_noremove.yul | 5 +- 7 files changed, 488 insertions(+), 503 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 273aa6b7917a..5da7b8ae6fa7 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -25,27 +25,40 @@ #include #include #include +#include #include #include +#include + using namespace std; using namespace solidity; using namespace solidity::yul; +// TODO this component does not handle reverting function calls specially. Is that OK? +// We should set m_activeStores to empty set for a reverting function call, like wo do with `leave`. + void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) { UnusedAssignEliminator rae{_context.dialect}; rae(_ast); - StatementRemover remover{rae.m_pendingRemovals}; + set toRemove; + for (Statement const* unusedStore: rae.m_allStores - rae.m_usedStores) + if (SideEffectsCollector{_context.dialect, *std::get(*unusedStore).value}.movable()) + toRemove.insert(unusedStore); + else + cerr << "not used because not movable" << endl; + + StatementRemover remover{toRemove}; remover(_ast); } void UnusedAssignEliminator::operator()(Identifier const& _identifier) { - changeUndecidedTo(_identifier.name, State::Used); + markUsed(_identifier.name); } void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDeclaration) @@ -59,10 +72,10 @@ void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDecl void UnusedAssignEliminator::operator()(Assignment const& _assignment) { visit(*_assignment.value); - for (auto const& var: _assignment.variableNames) - changeUndecidedTo(var.name, State::Unused); + // Do not visit the variables because they are Identifiers } + void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefinition) { ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {}); @@ -77,7 +90,7 @@ void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefin void UnusedAssignEliminator::operator()(Leave const&) { for (YulString name: m_returnVariables) - changeUndecidedTo(name, State::Used); + markUsed(name); } void UnusedAssignEliminator::operator()(Block const& _block) @@ -86,8 +99,10 @@ void UnusedAssignEliminator::operator()(Block const& _block) UnusedStoreBase::operator()(_block); - for (auto const& var: m_declaredVariables) - finalize(var, State::Unused); + for (auto const& statement: _block.statements) + if (auto const* varDecl = get_if(&statement)) + for (auto const& var: varDecl->variables) + m_activeStores.erase(var.name); } void UnusedAssignEliminator::visit(Statement const& _statement) @@ -95,63 +110,53 @@ void UnusedAssignEliminator::visit(Statement const& _statement) UnusedStoreBase::visit(_statement); if (auto const* assignment = get_if(&_statement)) - if (assignment->variableNames.size() == 1) - // Default-construct it in "Undecided" state if it does not yet exist. - m_stores[assignment->variableNames.front().name][&_statement]; + { + // TODO is it OK to do this for multi-assignments? I guess so because it is enough if + // one of them is used. + m_allStores.insert(&_statement); + for (auto const& var: assignment->variableNames) + m_activeStores[var.name] = {&_statement}; + } + +// cerr << "After " << std::visit(AsmPrinter{}, _statement) << endl; +// for (auto&& [var, assigns]: m_activeStores) +// { +// cerr << " " << var.str() << ":" << endl; +// for (auto const& assign: assigns) +// cerr << " " << std::visit(AsmPrinter{}, *assign) << endl; +// } } -void UnusedAssignEliminator::shortcutNestedLoop(TrackedStores const& _zeroRuns) +void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) { // Shortcut to avoid horrible runtime: // Change all assignments that were newly introduced in the for loop to "used". // We do not have to do that with the "break" or "continue" paths, because // they will be joined later anyway. // TODO parallel traversal might be more efficient here. - for (auto& [variable, stores]: m_stores) + + // TODO is this correct? + + for (auto& [variable, stores]: m_activeStores) for (auto& assignment: stores) { auto zeroIt = _zeroRuns.find(variable); - if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment.first)) + if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; - assignment.second = State::Value::Used; + m_usedStores.insert(assignment); } } void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const& _functionDefinition) { - for (auto const& param: _functionDefinition.parameters) - finalize(param.name, State::Unused); for (auto const& retParam: _functionDefinition.returnVariables) - finalize(retParam.name, State::Used); + markUsed(retParam.name); } -void UnusedAssignEliminator::changeUndecidedTo(YulString _variable, UnusedAssignEliminator::State _newState) +void UnusedAssignEliminator::markUsed(YulString _variable) { - for (auto& assignment: m_stores[_variable]) - if (assignment.second == State::Undecided) - assignment.second = _newState; -} - -void UnusedAssignEliminator::finalize(YulString _variable, UnusedAssignEliminator::State _finalState) -{ - std::map stores = std::move(m_stores[_variable]); - m_stores.erase(_variable); - - for (auto& breakAssignments: m_forLoopInfo.pendingBreakStmts) - { - util::joinMap(stores, std::move(breakAssignments[_variable]), State::join); - breakAssignments.erase(_variable); - } - for (auto& continueAssignments: m_forLoopInfo.pendingContinueStmts) - { - util::joinMap(stores, std::move(continueAssignments[_variable]), State::join); - continueAssignments.erase(_variable); - } - - for (auto&& [statement, state]: stores) - if ( - (state == State::Unused || (state == State::Undecided && _finalState == State::Unused)) && - SideEffectsCollector{m_dialect, *std::get(*statement).value}.movable() - ) - m_pendingRemovals.insert(statement); + for (auto& assignment: m_activeStores[_variable]) + m_usedStores.insert(assignment); + // TODO is this correct? + m_activeStores.erase(_variable); } diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index b5a2998a1734..d90278b77223 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -126,15 +126,10 @@ class UnusedAssignEliminator: public UnusedStoreBase void visit(Statement const& _statement) override; private: - void shortcutNestedLoop(TrackedStores const& _beforeLoop) override; + void shortcutNestedLoop(ActiveStores const& _beforeLoop) override; void finalizeFunctionDefinition(FunctionDefinition const& _functionDefinition) override; - void changeUndecidedTo(YulString _variable, State _newState); - /// 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 TrackedStores. - void finalize(YulString _variable, State _finalState); - + void markUsed(YulString _variable); std::set m_declaredVariables; std::set m_returnVariables; diff --git a/libyul/optimiser/UnusedStoreBase.cpp b/libyul/optimiser/UnusedStoreBase.cpp index 49f550842747..2f1c0a5da023 100644 --- a/libyul/optimiser/UnusedStoreBase.cpp +++ b/libyul/optimiser/UnusedStoreBase.cpp @@ -37,41 +37,41 @@ void UnusedStoreBase::operator()(If const& _if) { visit(*_if.condition); - TrackedStores skipBranch{m_stores}; + ActiveStores skipBranch{m_activeStores}; (*this)(_if.body); - merge(m_stores, std::move(skipBranch)); + merge(m_activeStores, std::move(skipBranch)); } void UnusedStoreBase::operator()(Switch const& _switch) { visit(*_switch.expression); - TrackedStores const preState{m_stores}; + ActiveStores const preState{m_activeStores}; bool hasDefault = false; - vector branches; + vector branches; for (auto const& c: _switch.cases) { if (!c.value) hasDefault = true; (*this)(c.body); - branches.emplace_back(std::move(m_stores)); - m_stores = preState; + branches.emplace_back(std::move(m_activeStores)); + m_activeStores = preState; } if (hasDefault) { - m_stores = std::move(branches.back()); + m_activeStores = std::move(branches.back()); branches.pop_back(); } for (auto& branch: branches) - merge(m_stores, std::move(branch)); + merge(m_activeStores, std::move(branch)); } void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition) { - ScopedSaveAndRestore outerAssignments(m_stores, {}); + ScopedSaveAndRestore outerAssignments(m_activeStores, {}); ScopedSaveAndRestore forLoopInfo(m_forLoopInfo, {}); ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, 0); @@ -94,10 +94,10 @@ void UnusedStoreBase::operator()(ForLoop const& _forLoop) visit(*_forLoop.condition); - TrackedStores zeroRuns{m_stores}; + ActiveStores zeroRuns{m_activeStores}; (*this)(_forLoop.body); - merge(m_stores, std::move(m_forLoopInfo.pendingContinueStmts)); + merge(m_activeStores, std::move(m_forLoopInfo.pendingContinueStmts)); m_forLoopInfo.pendingContinueStmts = {}; (*this)(_forLoop.post); @@ -106,54 +106,54 @@ void UnusedStoreBase::operator()(ForLoop const& _forLoop) if (m_forLoopNestingDepth < 6) { // Do the second run only for small nesting depths to avoid horrible runtime. - TrackedStores oneRun{m_stores}; + ActiveStores oneRun{m_activeStores}; (*this)(_forLoop.body); - merge(m_stores, std::move(m_forLoopInfo.pendingContinueStmts)); + merge(m_activeStores, std::move(m_forLoopInfo.pendingContinueStmts)); m_forLoopInfo.pendingContinueStmts.clear(); (*this)(_forLoop.post); visit(*_forLoop.condition); // Order of merging does not matter because "max" is commutative and associative. - merge(m_stores, std::move(oneRun)); + merge(m_activeStores, std::move(oneRun)); } else // Shortcut to avoid horrible runtime. shortcutNestedLoop(zeroRuns); // Order of merging does not matter because "max" is commutative and associative. - merge(m_stores, std::move(zeroRuns)); - merge(m_stores, std::move(m_forLoopInfo.pendingBreakStmts)); + merge(m_activeStores, std::move(zeroRuns)); + merge(m_activeStores, std::move(m_forLoopInfo.pendingBreakStmts)); m_forLoopInfo.pendingBreakStmts.clear(); } void UnusedStoreBase::operator()(Break const&) { - m_forLoopInfo.pendingBreakStmts.emplace_back(std::move(m_stores)); - m_stores.clear(); + m_forLoopInfo.pendingBreakStmts.emplace_back(std::move(m_activeStores)); + m_activeStores.clear(); } void UnusedStoreBase::operator()(Continue const&) { - m_forLoopInfo.pendingContinueStmts.emplace_back(std::move(m_stores)); - m_stores.clear(); + m_forLoopInfo.pendingContinueStmts.emplace_back(std::move(m_activeStores)); + m_activeStores.clear(); } -void UnusedStoreBase::merge(TrackedStores& _target, TrackedStores&& _other) +void UnusedStoreBase::merge(ActiveStores& _target, ActiveStores&& _other) { util::joinMap(_target, std::move(_other), []( - map& _assignmentHere, - map&& _assignmentThere + set& _storesHere, + set&& _storesThere ) { - return util::joinMap(_assignmentHere, std::move(_assignmentThere), State::join); + _storesHere += _storesThere; }); } -void UnusedStoreBase::merge(TrackedStores& _target, vector&& _source) +void UnusedStoreBase::merge(ActiveStores& _target, vector&& _source) { - for (TrackedStores& ts: _source) + for (ActiveStores& ts: _source) merge(_target, std::move(ts)); _source.clear(); } diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 15dccb04a2d2..35989da014c1 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -57,28 +57,12 @@ class UnusedStoreBase: public ASTWalker void operator()(Continue const&) override; protected: - class State - { - public: - enum Value { Unused, Undecided, Used }; - State(Value _value = Undecided): m_value(_value) {} - inline bool operator==(State _other) const { return m_value == _other.m_value; } - inline bool operator!=(State _other) const { return !operator==(_other); } - static inline void join(State& _a, State const& _b) - { - // Using "max" works here because of the order of the values in the enum. - _a.m_value = Value(std::max(int(_a.m_value), int(_b.m_value))); - } - private: - Value m_value = Undecided; - }; - - using TrackedStores = std::map>; + using ActiveStores = std::map>; /// This function is called for a loop that is nested too deep to avoid /// horrible runtime and should just resolve the situation in a pragmatic /// and correct manner. - virtual void shortcutNestedLoop(TrackedStores const& _beforeLoop) = 0; + virtual void shortcutNestedLoop(ActiveStores const& _beforeLoop) = 0; /// This function is called right before the scoped restore of the function definition. virtual void finalizeFunctionDefinition(FunctionDefinition const& /*_functionDefinition*/) {} @@ -86,20 +70,24 @@ class UnusedStoreBase: public ASTWalker /// Joins the assignment mapping of @a _source into @a _target according to the rules laid out /// above. /// Will destroy @a _source. - static void merge(TrackedStores& _target, TrackedStores&& _source); - static void merge(TrackedStores& _target, std::vector&& _source); + static void merge(ActiveStores& _target, ActiveStores&& _source); + static void merge(ActiveStores& _target, std::vector&& _source); Dialect const& m_dialect; - std::set m_pendingRemovals; - TrackedStores m_stores; + /// Set of all stores encountered during the traversal + std::set m_allStores; + /// Set of stores that are marked as being used. + std::set m_usedStores; + /// Active (undecided) stores in the current branch. + ActiveStores m_activeStores; /// Working data for traversing for-loops. struct ForLoopInfo { /// Tracked assignment states for each break statement. - std::vector pendingBreakStmts; + std::vector pendingBreakStmts; /// Tracked assignment states for each continue statement. - std::vector pendingContinueStmts; + std::vector pendingContinueStmts; }; ForLoopInfo m_forLoopInfo; size_t m_forLoopNestingDepth = 0; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 2e96be2e703a..aca7dd9078d6 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -50,359 +50,359 @@ static string const one{"@ 1"}; static string const thirtyTwo{"@ 32"}; -void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) +void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_ast*/) { - map functionSideEffects = SideEffectsPropagator::sideEffects( - _context.dialect, - CallGraphGenerator::callGraph(_ast) - ); - - SSAValueTracker ssaValues; - ssaValues(_ast); - map values; - for (auto const& [name, expression]: ssaValues.values()) - values[name] = AssignedValue{expression, {}}; - Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; - Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; - Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; - values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; - values[YulString{one}] = AssignedValue{&oneLiteral, {}}; - values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; - - bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); - UnusedStoreEliminator rse{ - _context.dialect, - functionSideEffects, - ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), - values, - ignoreMemory - }; - rse(_ast); - if ( - auto evmDialect = dynamic_cast(&_context.dialect); - evmDialect && evmDialect->providesObjectAccess() - ) - rse.changeUndecidedTo(State::Unused, Location::Memory); - else - rse.changeUndecidedTo(State::Used, Location::Memory); - rse.changeUndecidedTo(State::Used, Location::Storage); - rse.scheduleUnusedForDeletion(); - - StatementRemover remover(rse.m_pendingRemovals); - remover(_ast); +// map functionSideEffects = SideEffectsPropagator::sideEffects( +// _context.dialect, +// CallGraphGenerator::callGraph(_ast) +// ); + +// SSAValueTracker ssaValues; +// ssaValues(_ast); +// map values; +// for (auto const& [name, expression]: ssaValues.values()) +// values[name] = AssignedValue{expression, {}}; +// Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; +// Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; +// Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; +// values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; +// values[YulString{one}] = AssignedValue{&oneLiteral, {}}; +// values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; + +// bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); +// UnusedStoreEliminator rse{ +// _context.dialect, +// functionSideEffects, +// ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), +// values, +// ignoreMemory +// }; +// rse(_ast); +// if ( +// auto evmDialect = dynamic_cast(&_context.dialect); +// evmDialect && evmDialect->providesObjectAccess() +// ) +// rse.changeUndecidedTo(State::Unused, Location::Memory); +// else +// rse.changeUndecidedTo(State::Used, Location::Memory); +// rse.changeUndecidedTo(State::Used, Location::Storage); +// rse.scheduleUnusedForDeletion(); + +// StatementRemover remover(rse.m_pendingRemovals); +// remover(_ast); } -void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) -{ - UnusedStoreBase::operator()(_functionCall); - - for (Operation const& op: operationsFromFunctionCall(_functionCall)) - applyOperation(op); - - ControlFlowSideEffects sideEffects; - if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) - sideEffects = builtin->controlFlowSideEffects; - else - sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); - - if (sideEffects.canTerminate) - changeUndecidedTo(State::Used, Location::Storage); - if (!sideEffects.canContinue) - { - changeUndecidedTo(State::Unused, Location::Memory); - if (!sideEffects.canTerminate) - changeUndecidedTo(State::Unused, Location::Storage); - } -} - -void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) -{ - ScopedSaveAndRestore storeOperations(m_storeOperations, {}); - UnusedStoreBase::operator()(_functionDefinition); -} - - -void UnusedStoreEliminator::operator()(Leave const&) -{ - changeUndecidedTo(State::Used); -} - -void UnusedStoreEliminator::visit(Statement const& _statement) -{ - using evmasm::Instruction; - - UnusedStoreBase::visit(_statement); - - auto const* exprStatement = get_if(&_statement); - if (!exprStatement) - return; - - FunctionCall const* funCall = get_if(&exprStatement->expression); - yulAssert(funCall); - optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); - if (!instruction) - return; - - if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { - return get_if(&_expr) || get_if(&_expr); - })) - return; - - // We determine if this is a store instruction without additional side-effects - // both by querying a combination of semantic information and by listing the instructions. - // This way the assert below should be triggered on any change. - using evmasm::SemanticInformation; - bool isStorageWrite = (*instruction == Instruction::SSTORE); - bool isMemoryWrite = - *instruction == Instruction::EXTCODECOPY || - *instruction == Instruction::CODECOPY || - *instruction == Instruction::CALLDATACOPY || - *instruction == Instruction::RETURNDATACOPY || - *instruction == Instruction::MSTORE || - *instruction == Instruction::MSTORE8; - bool isCandidateForRemoval = - SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( - SemanticInformation::storage(*instruction) == SemanticInformation::Write || - (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) - ); - yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); - if (isCandidateForRemoval) - { - State initialState = State::Undecided; - if (*instruction == Instruction::RETURNDATACOPY) - { - initialState = State::Used; - auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); - auto length = identifierNameIfSSA(funCall->arguments.at(2)); - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - if (length && startOffset) - { - FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); - if ( - knowledge.knownToBeZero(*startOffset) && - lengthCall && - toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE - ) - initialState = State::Undecided; - } - } - m_stores[YulString{}].insert({&_statement, initialState}); - vector operations = operationsFromFunctionCall(*funCall); - yulAssert(operations.size() == 1, ""); - m_storeOperations[&_statement] = std::move(operations.front()); - } -} - -void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) -{ - changeUndecidedTo(State::Used); - scheduleUnusedForDeletion(); -} - -vector UnusedStoreEliminator::operationsFromFunctionCall( - FunctionCall const& _functionCall -) const -{ - using evmasm::Instruction; - - YulString functionName = _functionCall.functionName.name; - SideEffects sideEffects; - if (BuiltinFunction const* f = m_dialect.builtin(functionName)) - sideEffects = f->sideEffects; - else - sideEffects = m_functionSideEffects.at(functionName); - - optional instruction = toEVMInstruction(m_dialect, functionName); - if (!instruction) - { - vector result; - // Unknown read is worse than unknown write. - if (sideEffects.memory != SideEffects::Effect::None) - result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); - if (sideEffects.storage != SideEffects::Effect::None) - result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); - return result; - } - - using evmasm::SemanticInformation; - - return util::applyMap( - SemanticInformation::readWriteOperations(*instruction), - [&](SemanticInformation::Operation const& _op) -> Operation - { - yulAssert(!(_op.lengthParameter && _op.lengthConstant)); - yulAssert(_op.effect != Effect::None); - Operation ourOp{_op.location, _op.effect, {}, {}}; - if (_op.startParameter) - ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); - if (_op.lengthParameter) - ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); - if (_op.lengthConstant) - switch (*_op.lengthConstant) - { - case 1: ourOp.length = YulString(one); break; - case 32: ourOp.length = YulString(thirtyTwo); break; - default: yulAssert(false); - } - return ourOp; - } - ); -} - -void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) -{ - for (auto& [statement, state]: m_stores[YulString{}]) - if (state == State::Undecided) - { - Operation const& storeOperation = m_storeOperations.at(statement); - if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) - state = State::Used; - else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) - state = State::Unused; - } -} - -bool UnusedStoreEliminator::knownUnrelated( - UnusedStoreEliminator::Operation const& _op1, - UnusedStoreEliminator::Operation const& _op2 -) const -{ - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - - if (_op1.location != _op2.location) - return true; - if (_op1.location == Location::Storage) - { - if (_op1.start && _op2.start) - { - yulAssert( - _op1.length && - _op2.length && - knowledge.valueIfKnownConstant(*_op1.length) == 1 && - knowledge.valueIfKnownConstant(*_op2.length) == 1 - ); - return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); - } - } - else - { - yulAssert(_op1.location == Location::Memory, ""); - if ( - (_op1.length && knowledge.knownToBeZero(*_op1.length)) || - (_op2.length && knowledge.knownToBeZero(*_op2.length)) - ) - return true; - - if (_op1.start && _op1.length && _op2.start) - { - optional length1 = knowledge.valueIfKnownConstant(*_op1.length); - optional start1 = knowledge.valueIfKnownConstant(*_op1.start); - optional start2 = knowledge.valueIfKnownConstant(*_op2.start); - if ( - (length1 && start1 && start2) && - *start1 + *length1 >= *start1 && // no overflow - *start1 + *length1 <= *start2 - ) - return true; - } - if (_op2.start && _op2.length && _op1.start) - { - optional length2 = knowledge.valueIfKnownConstant(*_op2.length); - optional start2 = knowledge.valueIfKnownConstant(*_op2.start); - optional start1 = knowledge.valueIfKnownConstant(*_op1.start); - if ( - (length2 && start2 && start1) && - *start2 + *length2 >= *start2 && // no overflow - *start2 + *length2 <= *start1 - ) - return true; - } - - if (_op1.start && _op1.length && _op2.start && _op2.length) - { - optional length1 = knowledge.valueIfKnownConstant(*_op1.length); - optional length2 = knowledge.valueIfKnownConstant(*_op2.length); - if ( - (length1 && *length1 <= 32) && - (length2 && *length2 <= 32) && - knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) - ) - return true; - } - } - - return false; -} - -bool UnusedStoreEliminator::knownCovered( - UnusedStoreEliminator::Operation const& _covered, - UnusedStoreEliminator::Operation const& _covering -) const -{ - if (_covered.location != _covering.location) - return false; - if ( - (_covered.start && _covered.start == _covering.start) && - (_covered.length && _covered.length == _covering.length) - ) - return true; - if (_covered.location == Location::Memory) - { - KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - - if (_covered.length && knowledge.knownToBeZero(*_covered.length)) - return true; - - // Condition (i = cover_i_ng, e = cover_e_d): - // i.start <= e.start && e.start + e.length <= i.start + i.length - if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) - return false; - optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); - optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); - if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) - if (coveredLength && coveringLength && *coveredLength <= *coveringLength) - return true; - optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); - optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); - if (coveredStart && coveringStart && coveredLength && coveringLength) - if ( - *coveringStart <= *coveredStart && - *coveringStart + *coveringLength >= *coveringStart && // no overflow - *coveredStart + *coveredLength >= *coveredStart && // no overflow - *coveredStart + *coveredLength <= *coveringStart + *coveringLength - ) - return true; - - // TODO for this we probably need a non-overflow assumption as above. - // Condition (i = cover_i_ng, e = cover_e_d): - // i.start <= e.start && e.start + e.length <= i.start + i.length - } - return false; -} - -void UnusedStoreEliminator::changeUndecidedTo( - State _newState, - optional _onlyLocation) -{ - for (auto& [statement, state]: m_stores[YulString{}]) - if ( - state == State::Undecided && - (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) - ) - state = _newState; -} - -optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const -{ - if (Identifier const* identifier = get_if(&_expression)) - if (m_ssaValues.count(identifier->name)) - return {identifier->name}; - return nullopt; -} - -void UnusedStoreEliminator::scheduleUnusedForDeletion() -{ - for (auto const& [statement, state]: m_stores[YulString{}]) - if (state == State::Unused) - m_pendingRemovals.insert(statement); -} +//void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) +//{ +// UnusedStoreBase::operator()(_functionCall); + +// for (Operation const& op: operationsFromFunctionCall(_functionCall)) +// applyOperation(op); + +// ControlFlowSideEffects sideEffects; +// if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) +// sideEffects = builtin->controlFlowSideEffects; +// else +// sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); + +// if (sideEffects.canTerminate) +// changeUndecidedTo(State::Used, Location::Storage); +// if (!sideEffects.canContinue) +// { +// changeUndecidedTo(State::Unused, Location::Memory); +// if (!sideEffects.canTerminate) +// changeUndecidedTo(State::Unused, Location::Storage); +// } +//} + +//void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) +//{ +// ScopedSaveAndRestore storeOperations(m_storeOperations, {}); +// UnusedStoreBase::operator()(_functionDefinition); +//} + + +//void UnusedStoreEliminator::operator()(Leave const&) +//{ +// changeUndecidedTo(State::Used); +//} + +//void UnusedStoreEliminator::visit(Statement const& _statement) +//{ +// using evmasm::Instruction; + +// UnusedStoreBase::visit(_statement); + +// auto const* exprStatement = get_if(&_statement); +// if (!exprStatement) +// return; + +// FunctionCall const* funCall = get_if(&exprStatement->expression); +// yulAssert(funCall); +// optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); +// if (!instruction) +// return; + +// if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { +// return get_if(&_expr) || get_if(&_expr); +// })) +// return; + +// // We determine if this is a store instruction without additional side-effects +// // both by querying a combination of semantic information and by listing the instructions. +// // This way the assert below should be triggered on any change. +// using evmasm::SemanticInformation; +// bool isStorageWrite = (*instruction == Instruction::SSTORE); +// bool isMemoryWrite = +// *instruction == Instruction::EXTCODECOPY || +// *instruction == Instruction::CODECOPY || +// *instruction == Instruction::CALLDATACOPY || +// *instruction == Instruction::RETURNDATACOPY || +// *instruction == Instruction::MSTORE || +// *instruction == Instruction::MSTORE8; +// bool isCandidateForRemoval = +// SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( +// SemanticInformation::storage(*instruction) == SemanticInformation::Write || +// (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) +// ); +// yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); +// if (isCandidateForRemoval) +// { +// State initialState = State::Undecided; +// if (*instruction == Instruction::RETURNDATACOPY) +// { +// initialState = State::Used; +// auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); +// auto length = identifierNameIfSSA(funCall->arguments.at(2)); +// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); +// if (length && startOffset) +// { +// FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); +// if ( +// knowledge.knownToBeZero(*startOffset) && +// lengthCall && +// toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE +// ) +// initialState = State::Undecided; +// } +// } +// m_activeStores[YulString{}].insert({&_statement, initialState}); +// vector operations = operationsFromFunctionCall(*funCall); +// yulAssert(operations.size() == 1, ""); +// m_storeOperations[&_statement] = std::move(operations.front()); +// } +//} + +//void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) +//{ +// changeUndecidedTo(State::Used); +// scheduleUnusedForDeletion(); +//} + +//vector UnusedStoreEliminator::operationsFromFunctionCall( +// FunctionCall const& _functionCall +//) const +//{ +// using evmasm::Instruction; + +// YulString functionName = _functionCall.functionName.name; +// SideEffects sideEffects; +// if (BuiltinFunction const* f = m_dialect.builtin(functionName)) +// sideEffects = f->sideEffects; +// else +// sideEffects = m_functionSideEffects.at(functionName); + +// optional instruction = toEVMInstruction(m_dialect, functionName); +// if (!instruction) +// { +// vector result; +// // Unknown read is worse than unknown write. +// if (sideEffects.memory != SideEffects::Effect::None) +// result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); +// if (sideEffects.storage != SideEffects::Effect::None) +// result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); +// return result; +// } + +// using evmasm::SemanticInformation; + +// return util::applyMap( +// SemanticInformation::readWriteOperations(*instruction), +// [&](SemanticInformation::Operation const& _op) -> Operation +// { +// yulAssert(!(_op.lengthParameter && _op.lengthConstant)); +// yulAssert(_op.effect != Effect::None); +// Operation ourOp{_op.location, _op.effect, {}, {}}; +// if (_op.startParameter) +// ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); +// if (_op.lengthParameter) +// ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); +// if (_op.lengthConstant) +// switch (*_op.lengthConstant) +// { +// case 1: ourOp.length = YulString(one); break; +// case 32: ourOp.length = YulString(thirtyTwo); break; +// default: yulAssert(false); +// } +// return ourOp; +// } +// ); +//} + +//void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) +//{ +// for (auto& [statement, state]: m_activeStores[YulString{}]) +// if (state == State::Undecided) +// { +// Operation const& storeOperation = m_storeOperations.at(statement); +// if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) +// state = State::Used; +// else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) +// state = State::Unused; +// } +//} + +//bool UnusedStoreEliminator::knownUnrelated( +// UnusedStoreEliminator::Operation const& _op1, +// UnusedStoreEliminator::Operation const& _op2 +//) const +//{ +// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + +// if (_op1.location != _op2.location) +// return true; +// if (_op1.location == Location::Storage) +// { +// if (_op1.start && _op2.start) +// { +// yulAssert( +// _op1.length && +// _op2.length && +// knowledge.valueIfKnownConstant(*_op1.length) == 1 && +// knowledge.valueIfKnownConstant(*_op2.length) == 1 +// ); +// return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); +// } +// } +// else +// { +// yulAssert(_op1.location == Location::Memory, ""); +// if ( +// (_op1.length && knowledge.knownToBeZero(*_op1.length)) || +// (_op2.length && knowledge.knownToBeZero(*_op2.length)) +// ) +// return true; + +// if (_op1.start && _op1.length && _op2.start) +// { +// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); +// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); +// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); +// if ( +// (length1 && start1 && start2) && +// *start1 + *length1 >= *start1 && // no overflow +// *start1 + *length1 <= *start2 +// ) +// return true; +// } +// if (_op2.start && _op2.length && _op1.start) +// { +// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); +// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); +// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); +// if ( +// (length2 && start2 && start1) && +// *start2 + *length2 >= *start2 && // no overflow +// *start2 + *length2 <= *start1 +// ) +// return true; +// } + +// if (_op1.start && _op1.length && _op2.start && _op2.length) +// { +// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); +// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); +// if ( +// (length1 && *length1 <= 32) && +// (length2 && *length2 <= 32) && +// knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) +// ) +// return true; +// } +// } + +// return false; +//} + +//bool UnusedStoreEliminator::knownCovered( +// UnusedStoreEliminator::Operation const& _covered, +// UnusedStoreEliminator::Operation const& _covering +//) const +//{ +// if (_covered.location != _covering.location) +// return false; +// if ( +// (_covered.start && _covered.start == _covering.start) && +// (_covered.length && _covered.length == _covering.length) +// ) +// return true; +// if (_covered.location == Location::Memory) +// { +// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + +// if (_covered.length && knowledge.knownToBeZero(*_covered.length)) +// return true; + +// // Condition (i = cover_i_ng, e = cover_e_d): +// // i.start <= e.start && e.start + e.length <= i.start + i.length +// if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) +// return false; +// optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); +// optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); +// if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) +// if (coveredLength && coveringLength && *coveredLength <= *coveringLength) +// return true; +// optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); +// optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); +// if (coveredStart && coveringStart && coveredLength && coveringLength) +// if ( +// *coveringStart <= *coveredStart && +// *coveringStart + *coveringLength >= *coveringStart && // no overflow +// *coveredStart + *coveredLength >= *coveredStart && // no overflow +// *coveredStart + *coveredLength <= *coveringStart + *coveringLength +// ) +// return true; + +// // TODO for this we probably need a non-overflow assumption as above. +// // Condition (i = cover_i_ng, e = cover_e_d): +// // i.start <= e.start && e.start + e.length <= i.start + i.length +// } +// return false; +//} + +//void UnusedStoreEliminator::changeUndecidedTo( +// State _newState, +// optional _onlyLocation) +//{ +// for (auto& [statement, state]: m_activeStores[YulString{}]) +// if ( +// state == State::Undecided && +// (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) +// ) +// state = _newState; +//} + +//optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const +//{ +// if (Identifier const* identifier = get_if(&_expression)) +// if (m_ssaValues.count(identifier->name)) +// return {identifier->name}; +// return nullopt; +//} + +//void UnusedStoreEliminator::scheduleUnusedForDeletion() +//{ +// for (auto const& [statement, state]: m_activeStores[YulString{}]) +// if (state == State::Unused) +// m_pendingRemovals.insert(statement); +//} diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index 8b5bfd7b140b..40ddd73b8eea 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -64,63 +64,63 @@ class UnusedStoreEliminator: public UnusedStoreBase explicit UnusedStoreEliminator( Dialect const& _dialect, - std::map const& _functionSideEffects, - std::map _controlFlowSideEffects, - std::map const& _ssaValues, - bool _ignoreMemory + std::map const& ,//_functionSideEffects, + std::map,// _controlFlowSideEffects, + std::map const&,// _ssaValues, + bool// _ignoreMemory ): - UnusedStoreBase(_dialect), - m_ignoreMemory(_ignoreMemory), - m_functionSideEffects(_functionSideEffects), - m_controlFlowSideEffects(_controlFlowSideEffects), - m_ssaValues(_ssaValues) + UnusedStoreBase(_dialect)//, +// m_ignoreMemory(_ignoreMemory), +// m_functionSideEffects(_functionSideEffects), +// m_controlFlowSideEffects(_controlFlowSideEffects), +// m_ssaValues(_ssaValues) {} - using UnusedStoreBase::operator(); - void operator()(FunctionCall const& _functionCall) override; - void operator()(FunctionDefinition const&) override; - void operator()(Leave const&) override; - - using UnusedStoreBase::visit; - void visit(Statement const& _statement) override; - - using Location = evmasm::SemanticInformation::Location; - using Effect = evmasm::SemanticInformation::Effect; - struct Operation - { - Location location; - Effect effect; - /// Start of affected area. Unknown if not provided. - std::optional start; - /// Length of affected area, unknown if not provided. - /// Unused for storage. - std::optional length; - }; +// using UnusedStoreBase::operator(); +// void operator()(FunctionCall const& _functionCall) override; +// void operator()(FunctionDefinition const&) override; +// void operator()(Leave const&) override; + +// using UnusedStoreBase::visit; +// void visit(Statement const& _statement) override; + +// using Location = evmasm::SemanticInformation::Location; +// using Effect = evmasm::SemanticInformation::Effect; +// struct Operation +// { +// Location location; +// Effect effect; +// /// Start of affected area. Unknown if not provided. +// std::optional start; +// /// Length of affected area, unknown if not provided. +// /// Unused for storage. +// std::optional length; +// }; private: - void shortcutNestedLoop(TrackedStores const&) override + void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. - changeUndecidedTo(State::Used); +// changeUndecidedTo(State::Used); } - void finalizeFunctionDefinition(FunctionDefinition const&) override; +// void finalizeFunctionDefinition(FunctionDefinition const&) override; - std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; - void applyOperation(Operation const& _operation); - bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; - bool knownCovered(Operation const& _covered, Operation const& _covering) const; +// std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; +// void applyOperation(Operation const& _operation); +// bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; +// bool knownCovered(Operation const& _covered, Operation const& _covering) const; - void changeUndecidedTo(State _newState, std::optional _onlyLocation = std::nullopt); - void scheduleUnusedForDeletion(); +// void changeUndecidedTo(State _newState, std::optional _onlyLocation = std::nullopt); +// void scheduleUnusedForDeletion(); - std::optional identifierNameIfSSA(Expression const& _expression) const; +// std::optional identifierNameIfSSA(Expression const& _expression) const; - bool const m_ignoreMemory; - std::map const& m_functionSideEffects; - std::map m_controlFlowSideEffects; - std::map const& m_ssaValues; +// bool const m_ignoreMemory; +// std::map const& m_functionSideEffects; +// std::map m_controlFlowSideEffects; +// std::map const& m_ssaValues; - std::map m_storeOperations; +// std::map m_storeOperations; }; } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul index 02a735f9e50f..c0a84c2450eb 100644 --- a/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/for_deep_noremove.yul @@ -46,10 +46,7 @@ // for { } 1 { } // { // for { } 1 { a := 10 } -// { -// b := 12 -// b := 11 -// } +// { b := 11 } // } // } // } From ddbcea047b9c3aacaa3c172f6e9768cba2aa739e Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 25 Nov 2022 14:20:38 +0100 Subject: [PATCH 11/26] some more work --- libyul/optimiser/UnusedAssignEliminator.cpp | 5 +- libyul/optimiser/UnusedStoreEliminator.cpp | 699 +++++++++--------- libyul/optimiser/UnusedStoreEliminator.h | 77 +- .../conditionally_assign_before_revert.yul | 21 + 4 files changed, 429 insertions(+), 373 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 5da7b8ae6fa7..eaa076fed3d5 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -47,6 +47,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) set toRemove; for (Statement const* unusedStore: rae.m_allStores - rae.m_usedStores) + // TODO this should also use user function side effects. + // Then we have to modify the multi-assign test (or verify that it is fine after all + // by adding a test where one var is used but not the other) if (SideEffectsCollector{_context.dialect, *std::get(*unusedStore).value}.movable()) toRemove.insert(unusedStore); else @@ -111,8 +114,6 @@ void UnusedAssignEliminator::visit(Statement const& _statement) if (auto const* assignment = get_if(&_statement)) { - // TODO is it OK to do this for multi-assignments? I guess so because it is enough if - // one of them is used. m_allStores.insert(&_statement); for (auto const& var: assignment->variableNames) m_activeStores[var.name] = {&_statement}; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index aca7dd9078d6..c9b5579c2990 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -52,123 +52,125 @@ static string const thirtyTwo{"@ 32"}; void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_ast*/) { -// map functionSideEffects = SideEffectsPropagator::sideEffects( -// _context.dialect, -// CallGraphGenerator::callGraph(_ast) -// ); - -// SSAValueTracker ssaValues; -// ssaValues(_ast); -// map values; -// for (auto const& [name, expression]: ssaValues.values()) -// values[name] = AssignedValue{expression, {}}; -// Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; -// Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; -// Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; -// values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; -// values[YulString{one}] = AssignedValue{&oneLiteral, {}}; -// values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; - -// bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); -// UnusedStoreEliminator rse{ -// _context.dialect, -// functionSideEffects, -// ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), -// values, -// ignoreMemory -// }; -// rse(_ast); -// if ( -// auto evmDialect = dynamic_cast(&_context.dialect); -// evmDialect && evmDialect->providesObjectAccess() -// ) -// rse.changeUndecidedTo(State::Unused, Location::Memory); -// else -// rse.changeUndecidedTo(State::Used, Location::Memory); -// rse.changeUndecidedTo(State::Used, Location::Storage); -// rse.scheduleUnusedForDeletion(); - -// StatementRemover remover(rse.m_pendingRemovals); -// remover(_ast); + map functionSideEffects = SideEffectsPropagator::sideEffects( + _context.dialect, + CallGraphGenerator::callGraph(_ast) + ); + + SSAValueTracker ssaValues; + ssaValues(_ast); + map values; + for (auto const& [name, expression]: ssaValues.values()) + values[name] = AssignedValue{expression, {}}; + Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; + Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; + Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; + values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; + values[YulString{one}] = AssignedValue{&oneLiteral, {}}; + values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; + + bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); + UnusedStoreEliminator rse{ + _context.dialect, + functionSideEffects, + ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), + values, + ignoreMemory + }; + rse(_ast); + if ( + auto evmDialect = dynamic_cast(&_context.dialect); + evmDialect && evmDialect->providesObjectAccess() + ) + { + } + else + rse.markActiveAsUsed(Location::Memory); + rse.markActiveAsUsed(Location::Storage); + rse.scheduleUnusedForDeletion(); + + StatementRemover remover(rse.m_pendingRemovals); + remover(_ast); } -//void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) -//{ -// UnusedStoreBase::operator()(_functionCall); - -// for (Operation const& op: operationsFromFunctionCall(_functionCall)) -// applyOperation(op); - -// ControlFlowSideEffects sideEffects; -// if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) -// sideEffects = builtin->controlFlowSideEffects; -// else -// sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); - -// if (sideEffects.canTerminate) -// changeUndecidedTo(State::Used, Location::Storage); -// if (!sideEffects.canContinue) -// { -// changeUndecidedTo(State::Unused, Location::Memory); -// if (!sideEffects.canTerminate) -// changeUndecidedTo(State::Unused, Location::Storage); -// } -//} - -//void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) -//{ -// ScopedSaveAndRestore storeOperations(m_storeOperations, {}); -// UnusedStoreBase::operator()(_functionDefinition); -//} - - -//void UnusedStoreEliminator::operator()(Leave const&) -//{ -// changeUndecidedTo(State::Used); -//} - -//void UnusedStoreEliminator::visit(Statement const& _statement) -//{ -// using evmasm::Instruction; - -// UnusedStoreBase::visit(_statement); - -// auto const* exprStatement = get_if(&_statement); -// if (!exprStatement) -// return; - -// FunctionCall const* funCall = get_if(&exprStatement->expression); -// yulAssert(funCall); -// optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); -// if (!instruction) -// return; - -// if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { -// return get_if(&_expr) || get_if(&_expr); -// })) -// return; - -// // We determine if this is a store instruction without additional side-effects -// // both by querying a combination of semantic information and by listing the instructions. -// // This way the assert below should be triggered on any change. -// using evmasm::SemanticInformation; -// bool isStorageWrite = (*instruction == Instruction::SSTORE); -// bool isMemoryWrite = -// *instruction == Instruction::EXTCODECOPY || -// *instruction == Instruction::CODECOPY || -// *instruction == Instruction::CALLDATACOPY || -// *instruction == Instruction::RETURNDATACOPY || -// *instruction == Instruction::MSTORE || -// *instruction == Instruction::MSTORE8; -// bool isCandidateForRemoval = -// SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( -// SemanticInformation::storage(*instruction) == SemanticInformation::Write || -// (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) -// ); -// yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); -// if (isCandidateForRemoval) -// { -// State initialState = State::Undecided; +void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) +{ + UnusedStoreBase::operator()(_functionCall); + + for (Operation const& op: operationsFromFunctionCall(_functionCall)) + applyOperation(op); + + ControlFlowSideEffects sideEffects; + if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) + sideEffects = builtin->controlFlowSideEffects; + else + sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); + + if (sideEffects.canTerminate) + markActiveAsUsed(Location::Storage); + if (!sideEffects.canContinue) + { + clearActive(Location::Memory); + if (!sideEffects.canTerminate) + clearActive(Location::Storage); + } +} + +void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) +{ + ScopedSaveAndRestore storeOperations(m_storeOperations, {}); + UnusedStoreBase::operator()(_functionDefinition); +} + + +void UnusedStoreEliminator::operator()(Leave const&) +{ + markActiveAsUsed(); +} + +void UnusedStoreEliminator::visit(Statement const& _statement) +{ + using evmasm::Instruction; + + UnusedStoreBase::visit(_statement); + + auto const* exprStatement = get_if(&_statement); + if (!exprStatement) + return; + + FunctionCall const* funCall = get_if(&exprStatement->expression); + yulAssert(funCall); + optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); + if (!instruction) + return; + + if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { + return get_if(&_expr) || get_if(&_expr); + })) + return; + + // We determine if this is a store instruction without additional side-effects + // both by querying a combination of semantic information and by listing the instructions. + // This way the assert below should be triggered on any change. + using evmasm::SemanticInformation; + bool isStorageWrite = (*instruction == Instruction::SSTORE); + bool isMemoryWrite = + *instruction == Instruction::EXTCODECOPY || + *instruction == Instruction::CODECOPY || + *instruction == Instruction::CALLDATACOPY || + *instruction == Instruction::RETURNDATACOPY || + *instruction == Instruction::MSTORE || + *instruction == Instruction::MSTORE8; + bool isCandidateForRemoval = + SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( + SemanticInformation::storage(*instruction) == SemanticInformation::Write || + (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) + ); + yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); + if (isCandidateForRemoval) + { + // TODO what is this special case? + //State initialState = State::Undecided; // if (*instruction == Instruction::RETURNDATACOPY) // { // initialState = State::Used; @@ -186,223 +188,254 @@ void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_as // initialState = State::Undecided; // } // } -// m_activeStores[YulString{}].insert({&_statement, initialState}); -// vector operations = operationsFromFunctionCall(*funCall); -// yulAssert(operations.size() == 1, ""); -// m_storeOperations[&_statement] = std::move(operations.front()); -// } -//} - -//void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) -//{ -// changeUndecidedTo(State::Used); -// scheduleUnusedForDeletion(); -//} - -//vector UnusedStoreEliminator::operationsFromFunctionCall( -// FunctionCall const& _functionCall -//) const -//{ -// using evmasm::Instruction; - -// YulString functionName = _functionCall.functionName.name; -// SideEffects sideEffects; -// if (BuiltinFunction const* f = m_dialect.builtin(functionName)) -// sideEffects = f->sideEffects; -// else -// sideEffects = m_functionSideEffects.at(functionName); - -// optional instruction = toEVMInstruction(m_dialect, functionName); -// if (!instruction) -// { -// vector result; -// // Unknown read is worse than unknown write. -// if (sideEffects.memory != SideEffects::Effect::None) -// result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); -// if (sideEffects.storage != SideEffects::Effect::None) -// result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); -// return result; -// } - -// using evmasm::SemanticInformation; - -// return util::applyMap( -// SemanticInformation::readWriteOperations(*instruction), -// [&](SemanticInformation::Operation const& _op) -> Operation -// { -// yulAssert(!(_op.lengthParameter && _op.lengthConstant)); -// yulAssert(_op.effect != Effect::None); -// Operation ourOp{_op.location, _op.effect, {}, {}}; -// if (_op.startParameter) -// ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); -// if (_op.lengthParameter) -// ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); -// if (_op.lengthConstant) -// switch (*_op.lengthConstant) -// { -// case 1: ourOp.length = YulString(one); break; -// case 32: ourOp.length = YulString(thirtyTwo); break; -// default: yulAssert(false); -// } -// return ourOp; -// } -// ); -//} + m_allStores.insert(&_statement); + vector operations = operationsFromFunctionCall(*funCall); + yulAssert(operations.size() == 1, ""); + if (operations.front().location == Location::Storage) + m_activeStores["s"_yulstring].insert(&_statement); + else + m_activeStores["m"_yulstring].insert(&_statement); + m_storeOperations[&_statement] = std::move(operations.front()); + } +} -//void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) -//{ -// for (auto& [statement, state]: m_activeStores[YulString{}]) -// if (state == State::Undecided) -// { -// Operation const& storeOperation = m_storeOperations.at(statement); -// if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) -// state = State::Used; -// else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) -// state = State::Unused; -// } -//} - -//bool UnusedStoreEliminator::knownUnrelated( -// UnusedStoreEliminator::Operation const& _op1, -// UnusedStoreEliminator::Operation const& _op2 -//) const -//{ -// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - -// if (_op1.location != _op2.location) -// return true; -// if (_op1.location == Location::Storage) -// { -// if (_op1.start && _op2.start) -// { -// yulAssert( -// _op1.length && -// _op2.length && -// knowledge.valueIfKnownConstant(*_op1.length) == 1 && -// knowledge.valueIfKnownConstant(*_op2.length) == 1 -// ); -// return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); -// } -// } -// else -// { -// yulAssert(_op1.location == Location::Memory, ""); -// if ( -// (_op1.length && knowledge.knownToBeZero(*_op1.length)) || -// (_op2.length && knowledge.knownToBeZero(*_op2.length)) -// ) -// return true; - -// if (_op1.start && _op1.length && _op2.start) -// { -// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); -// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); -// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); -// if ( -// (length1 && start1 && start2) && -// *start1 + *length1 >= *start1 && // no overflow -// *start1 + *length1 <= *start2 -// ) -// return true; -// } -// if (_op2.start && _op2.length && _op1.start) -// { -// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); -// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); -// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); -// if ( -// (length2 && start2 && start1) && -// *start2 + *length2 >= *start2 && // no overflow -// *start2 + *length2 <= *start1 -// ) -// return true; -// } +void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) +{ + markActiveAsUsed(); + scheduleUnusedForDeletion(); +} -// if (_op1.start && _op1.length && _op2.start && _op2.length) -// { -// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); -// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); -// if ( -// (length1 && *length1 <= 32) && -// (length2 && *length2 <= 32) && -// knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) -// ) -// return true; -// } -// } - -// return false; -//} - -//bool UnusedStoreEliminator::knownCovered( -// UnusedStoreEliminator::Operation const& _covered, -// UnusedStoreEliminator::Operation const& _covering -//) const -//{ -// if (_covered.location != _covering.location) -// return false; -// if ( -// (_covered.start && _covered.start == _covering.start) && -// (_covered.length && _covered.length == _covering.length) -// ) -// return true; -// if (_covered.location == Location::Memory) -// { -// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); - -// if (_covered.length && knowledge.knownToBeZero(*_covered.length)) -// return true; - -// // Condition (i = cover_i_ng, e = cover_e_d): -// // i.start <= e.start && e.start + e.length <= i.start + i.length -// if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) -// return false; -// optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); -// optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); -// if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) -// if (coveredLength && coveringLength && *coveredLength <= *coveringLength) -// return true; -// optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); -// optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); -// if (coveredStart && coveringStart && coveredLength && coveringLength) -// if ( -// *coveringStart <= *coveredStart && -// *coveringStart + *coveringLength >= *coveringStart && // no overflow -// *coveredStart + *coveredLength >= *coveredStart && // no overflow -// *coveredStart + *coveredLength <= *coveringStart + *coveringLength -// ) -// return true; - -// // TODO for this we probably need a non-overflow assumption as above. -// // Condition (i = cover_i_ng, e = cover_e_d): -// // i.start <= e.start && e.start + e.length <= i.start + i.length -// } -// return false; -//} - -//void UnusedStoreEliminator::changeUndecidedTo( -// State _newState, -// optional _onlyLocation) -//{ -// for (auto& [statement, state]: m_activeStores[YulString{}]) -// if ( -// state == State::Undecided && -// (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) -// ) -// state = _newState; -//} - -//optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const -//{ -// if (Identifier const* identifier = get_if(&_expression)) -// if (m_ssaValues.count(identifier->name)) -// return {identifier->name}; -// return nullopt; -//} - -//void UnusedStoreEliminator::scheduleUnusedForDeletion() -//{ -// for (auto const& [statement, state]: m_activeStores[YulString{}]) -// if (state == State::Unused) -// m_pendingRemovals.insert(statement); -//} +vector UnusedStoreEliminator::operationsFromFunctionCall( + FunctionCall const& _functionCall +) const +{ + using evmasm::Instruction; + + YulString functionName = _functionCall.functionName.name; + SideEffects sideEffects; + if (BuiltinFunction const* f = m_dialect.builtin(functionName)) + sideEffects = f->sideEffects; + else + sideEffects = m_functionSideEffects.at(functionName); + + optional instruction = toEVMInstruction(m_dialect, functionName); + if (!instruction) + { + vector result; + // Unknown read is worse than unknown write. + if (sideEffects.memory != SideEffects::Effect::None) + result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); + if (sideEffects.storage != SideEffects::Effect::None) + result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); + return result; + } + + using evmasm::SemanticInformation; + + return util::applyMap( + SemanticInformation::readWriteOperations(*instruction), + [&](SemanticInformation::Operation const& _op) -> Operation + { + yulAssert(!(_op.lengthParameter && _op.lengthConstant)); + yulAssert(_op.effect != Effect::None); + Operation ourOp{_op.location, _op.effect, {}, {}}; + if (_op.startParameter) + ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); + if (_op.lengthParameter) + ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); + if (_op.lengthConstant) + switch (*_op.lengthConstant) + { + case 1: ourOp.length = YulString(one); break; + case 32: ourOp.length = YulString(thirtyTwo); break; + default: yulAssert(false); + } + return ourOp; + } + ); +} + +void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) +{ + // TODO only one will be relevant, depending on _operation.location + for (Statement const* statement: m_activeStores["s"_yulstring]) + { + Operation const& storeOperation = m_storeOperations.at(statement); + if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) + // TODO remove from active! + m_usedStores.insert(statement); + else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) + { + // TODO remove from active +// state = State::Unused; + } + } + for (Statement const* statement: m_activeStores["m"_yulstring]) + { + Operation const& storeOperation = m_storeOperations.at(statement); + if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) + // TODO remove from active! + m_usedStores.insert(statement); + else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) + { + // TODO remove from active +// state = State::Unused; + } + } +} + +bool UnusedStoreEliminator::knownUnrelated( + UnusedStoreEliminator::Operation const& _op1, + UnusedStoreEliminator::Operation const& _op2 +) const +{ + KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + + if (_op1.location != _op2.location) + return true; + if (_op1.location == Location::Storage) + { + if (_op1.start && _op2.start) + { + yulAssert( + _op1.length && + _op2.length && + knowledge.valueIfKnownConstant(*_op1.length) == 1 && + knowledge.valueIfKnownConstant(*_op2.length) == 1 + ); + return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); + } + } + else + { + yulAssert(_op1.location == Location::Memory, ""); + if ( + (_op1.length && knowledge.knownToBeZero(*_op1.length)) || + (_op2.length && knowledge.knownToBeZero(*_op2.length)) + ) + return true; + + if (_op1.start && _op1.length && _op2.start) + { + optional length1 = knowledge.valueIfKnownConstant(*_op1.length); + optional start1 = knowledge.valueIfKnownConstant(*_op1.start); + optional start2 = knowledge.valueIfKnownConstant(*_op2.start); + if ( + (length1 && start1 && start2) && + *start1 + *length1 >= *start1 && // no overflow + *start1 + *length1 <= *start2 + ) + return true; + } + if (_op2.start && _op2.length && _op1.start) + { + optional length2 = knowledge.valueIfKnownConstant(*_op2.length); + optional start2 = knowledge.valueIfKnownConstant(*_op2.start); + optional start1 = knowledge.valueIfKnownConstant(*_op1.start); + if ( + (length2 && start2 && start1) && + *start2 + *length2 >= *start2 && // no overflow + *start2 + *length2 <= *start1 + ) + return true; + } + + if (_op1.start && _op1.length && _op2.start && _op2.length) + { + optional length1 = knowledge.valueIfKnownConstant(*_op1.length); + optional length2 = knowledge.valueIfKnownConstant(*_op2.length); + if ( + (length1 && *length1 <= 32) && + (length2 && *length2 <= 32) && + knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) + ) + return true; + } + } + + return false; +} + +bool UnusedStoreEliminator::knownCovered( + UnusedStoreEliminator::Operation const& _covered, + UnusedStoreEliminator::Operation const& _covering +) const +{ + if (_covered.location != _covering.location) + return false; + if ( + (_covered.start && _covered.start == _covering.start) && + (_covered.length && _covered.length == _covering.length) + ) + return true; + if (_covered.location == Location::Memory) + { + KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + + if (_covered.length && knowledge.knownToBeZero(*_covered.length)) + return true; + + // Condition (i = cover_i_ng, e = cover_e_d): + // i.start <= e.start && e.start + e.length <= i.start + i.length + if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) + return false; + optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); + optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); + if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) + if (coveredLength && coveringLength && *coveredLength <= *coveringLength) + return true; + optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); + optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); + if (coveredStart && coveringStart && coveredLength && coveringLength) + if ( + *coveringStart <= *coveredStart && + *coveringStart + *coveringLength >= *coveringStart && // no overflow + *coveredStart + *coveredLength >= *coveredStart && // no overflow + *coveredStart + *coveredLength <= *coveringStart + *coveringLength + ) + return true; + + // TODO for this we probably need a non-overflow assumption as above. + // Condition (i = cover_i_ng, e = cover_e_d): + // i.start <= e.start && e.start + e.length <= i.start + i.length + } + return false; +} + +void UnusedStoreEliminator::markActiveAsUsed( + optional _onlyLocation) +{ + // TODO it might make sense to use YulString{"m"} and YulString{"s"} for memory and storage. + // BUT: Could be both! + for (Statement const* statement: m_activeStores[YulString{}]) + if (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) + m_usedStores.insert(statement); + // TODO and remove from active +} + +void UnusedStoreEliminator::changeUndecidedTo( + State _newState, + optional _onlyLocation) +{ + for (auto& [statement, state]: m_activeStores[YulString{}]) + if ( + state == State::Undecided && + (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) + ) + state = _newState; +} + +optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const +{ + if (Identifier const* identifier = get_if(&_expression)) + if (m_ssaValues.count(identifier->name)) + return {identifier->name}; + return nullopt; +} + +void UnusedStoreEliminator::scheduleUnusedForDeletion() +{ + for (auto const& [statement, state]: m_activeStores[YulString{}]) + if (state == State::Unused) + m_pendingRemovals.insert(statement); +} diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index 40ddd73b8eea..ce9c1fc1619e 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -70,57 +70,58 @@ class UnusedStoreEliminator: public UnusedStoreBase bool// _ignoreMemory ): UnusedStoreBase(_dialect)//, -// m_ignoreMemory(_ignoreMemory), -// m_functionSideEffects(_functionSideEffects), -// m_controlFlowSideEffects(_controlFlowSideEffects), -// m_ssaValues(_ssaValues) + m_ignoreMemory(_ignoreMemory), + m_functionSideEffects(_functionSideEffects), + m_controlFlowSideEffects(_controlFlowSideEffects), + m_ssaValues(_ssaValues) {} -// using UnusedStoreBase::operator(); -// void operator()(FunctionCall const& _functionCall) override; -// void operator()(FunctionDefinition const&) override; -// void operator()(Leave const&) override; - -// using UnusedStoreBase::visit; -// void visit(Statement const& _statement) override; - -// using Location = evmasm::SemanticInformation::Location; -// using Effect = evmasm::SemanticInformation::Effect; -// struct Operation -// { -// Location location; -// Effect effect; -// /// Start of affected area. Unknown if not provided. -// std::optional start; -// /// Length of affected area, unknown if not provided. -// /// Unused for storage. -// std::optional length; -// }; + using UnusedStoreBase::operator(); + void operator()(FunctionCall const& _functionCall) override; + void operator()(FunctionDefinition const&) override; + void operator()(Leave const&) override; + + using UnusedStoreBase::visit; + void visit(Statement const& _statement) override; + + using Location = evmasm::SemanticInformation::Location; + using Effect = evmasm::SemanticInformation::Effect; + struct Operation + { + Location location; + Effect effect; + /// Start of affected area. Unknown if not provided. + std::optional start; + /// Length of affected area, unknown if not provided. + /// Unused for storage. + std::optional length; + }; private: void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. -// changeUndecidedTo(State::Used); + changeUndecidedTo(State::Used); } -// void finalizeFunctionDefinition(FunctionDefinition const&) override; + void finalizeFunctionDefinition(FunctionDefinition const&) override; -// std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; -// void applyOperation(Operation const& _operation); -// bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; -// bool knownCovered(Operation const& _covered, Operation const& _covering) const; + std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; + void applyOperation(Operation const& _operation); + bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; + bool knownCovered(Operation const& _covered, Operation const& _covering) const; -// void changeUndecidedTo(State _newState, std::optional _onlyLocation = std::nullopt); -// void scheduleUnusedForDeletion(); + void markActiveAsUsed(std::optional _onlyLocation = std::nullopt); + void clearActive(std::optional _onlyLocation = std::nullopt); + void scheduleUnusedForDeletion(); -// std::optional identifierNameIfSSA(Expression const& _expression) const; + std::optional identifierNameIfSSA(Expression const& _expression) const; -// bool const m_ignoreMemory; -// std::map const& m_functionSideEffects; -// std::map m_controlFlowSideEffects; -// std::map const& m_ssaValues; + bool const m_ignoreMemory; + std::map const& m_functionSideEffects; + std::map m_controlFlowSideEffects; + std::map const& m_ssaValues; -// std::map m_storeOperations; + std::map m_storeOperations; }; } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul new file mode 100644 index 000000000000..88b12264ef0a --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul @@ -0,0 +1,21 @@ +{ + let a := calldataload(0) + if calldataload(1) { + // this can be removed + a := 2 + revert(0, 0) + } + sstore(0, a) +} +// ---- +// step: unusedAssignEliminator +// +// { +// let a := calldataload(0) +// if calldataload(1) +// { +// a := 2 +// revert(0, 0) +// } +// sstore(0, a) +// } From b09a8c62bbf4511ace3627bc07ecd85069ca9550 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 13:28:21 +0100 Subject: [PATCH 12/26] unused store --- libyul/optimiser/UnusedStoreEliminator.cpp | 79 +++++++++------------- libyul/optimiser/UnusedStoreEliminator.h | 19 +++--- 2 files changed, 43 insertions(+), 55 deletions(-) diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index c9b5579c2990..e2bbe7645a31 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -50,7 +50,7 @@ static string const one{"@ 1"}; static string const thirtyTwo{"@ 32"}; -void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_ast*/) +void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) { map functionSideEffects = SideEffectsPropagator::sideEffects( _context.dialect, @@ -87,9 +87,8 @@ void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_as else rse.markActiveAsUsed(Location::Memory); rse.markActiveAsUsed(Location::Storage); - rse.scheduleUnusedForDeletion(); - StatementRemover remover(rse.m_pendingRemovals); + StatementRemover remover{rse.m_allStores - rse.m_usedStores}; remover(_ast); } @@ -192,9 +191,9 @@ void UnusedStoreEliminator::visit(Statement const& _statement) vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); if (operations.front().location == Location::Storage) - m_activeStores["s"_yulstring].insert(&_statement); + activeStorageStores().insert(&_statement); else - m_activeStores["m"_yulstring].insert(&_statement); + activeMemoryStores().insert(&_statement); m_storeOperations[&_statement] = std::move(operations.front()); } } @@ -202,7 +201,6 @@ void UnusedStoreEliminator::visit(Statement const& _statement) void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) { markActiveAsUsed(); - scheduleUnusedForDeletion(); } vector UnusedStoreEliminator::operationsFromFunctionCall( @@ -257,31 +255,27 @@ vector UnusedStoreEliminator::operationsFromFu void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) { - // TODO only one will be relevant, depending on _operation.location - for (Statement const* statement: m_activeStores["s"_yulstring]) + set toRemove; + set& active = + _operation.location == Location::Storage ? + activeStorageStores() : + activeMemoryStores(); + + // TODO this loop could be done more efficiently - removing while iterating. + for (Statement const* statement: active) { Operation const& storeOperation = m_storeOperations.at(statement); if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) - // TODO remove from active! - m_usedStores.insert(statement); - else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) { - // TODO remove from active -// state = State::Unused; - } - } - for (Statement const* statement: m_activeStores["m"_yulstring]) - { - Operation const& storeOperation = m_storeOperations.at(statement); - if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) - // TODO remove from active! + // This store is read from, mark it as used and remove it from the active set. m_usedStores.insert(statement); - else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) - { - // TODO remove from active -// state = State::Unused; + toRemove.insert(statement); } + else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) + // This store is overwritten before being read, remove it from the active set. + toRemove.insert(statement); } + active -= toRemove; } bool UnusedStoreEliminator::knownUnrelated( @@ -403,26 +397,26 @@ bool UnusedStoreEliminator::knownCovered( } void UnusedStoreEliminator::markActiveAsUsed( - optional _onlyLocation) + optional _onlyLocation +) { - // TODO it might make sense to use YulString{"m"} and YulString{"s"} for memory and storage. - // BUT: Could be both! - for (Statement const* statement: m_activeStores[YulString{}]) - if (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) + if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) + for (Statement const* statement: activeMemoryStores()) m_usedStores.insert(statement); - // TODO and remove from active + if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) + for (Statement const* statement: activeStorageStores()) + m_usedStores.insert(statement); + clearActive(_onlyLocation); } -void UnusedStoreEliminator::changeUndecidedTo( - State _newState, - optional _onlyLocation) +void UnusedStoreEliminator::clearActive( + optional _onlyLocation +) { - for (auto& [statement, state]: m_activeStores[YulString{}]) - if ( - state == State::Undecided && - (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) - ) - state = _newState; + if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) + activeMemoryStores() = {}; + if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) + activeStorageStores() = {}; } optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const @@ -432,10 +426,3 @@ optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& return {identifier->name}; return nullopt; } - -void UnusedStoreEliminator::scheduleUnusedForDeletion() -{ - for (auto const& [statement, state]: m_activeStores[YulString{}]) - if (state == State::Unused) - m_pendingRemovals.insert(statement); -} diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index ce9c1fc1619e..94620ab7b5bc 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -49,8 +49,7 @@ struct AssignedValue; * to sstore, as we don't know whether the memory location will be read once we leave the function's scope, * so the statement will be removed only if all code code paths lead to a memory overwrite. * - * The m_store member of UnusedStoreBase is only used with the empty yul string - * as key in the first dimension. + * The m_store member of UnusedStoreBase uses the key "m" for memory ond "s" for storage stores. * * Best run in SSA form. * @@ -64,12 +63,12 @@ class UnusedStoreEliminator: public UnusedStoreBase explicit UnusedStoreEliminator( Dialect const& _dialect, - std::map const& ,//_functionSideEffects, - std::map,// _controlFlowSideEffects, - std::map const&,// _ssaValues, - bool// _ignoreMemory + std::map const& _functionSideEffects, + std::map _controlFlowSideEffects, + std::map const& _ssaValues, + bool _ignoreMemory ): - UnusedStoreBase(_dialect)//, + UnusedStoreBase(_dialect), m_ignoreMemory(_ignoreMemory), m_functionSideEffects(_functionSideEffects), m_controlFlowSideEffects(_controlFlowSideEffects), @@ -98,10 +97,13 @@ class UnusedStoreEliminator: public UnusedStoreBase }; private: + std::set& activeMemoryStores() { return m_activeStores["m"_yulstring]; } + std::set& activeStorageStores() { return m_activeStores["m"_yulstring]; } + void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. - changeUndecidedTo(State::Used); + markActiveAsUsed(); } void finalizeFunctionDefinition(FunctionDefinition const&) override; @@ -112,7 +114,6 @@ class UnusedStoreEliminator: public UnusedStoreBase void markActiveAsUsed(std::optional _onlyLocation = std::nullopt); void clearActive(std::optional _onlyLocation = std::nullopt); - void scheduleUnusedForDeletion(); std::optional identifierNameIfSSA(Expression const& _expression) const; From 1013419597c0f4db4cb35dc8d0f668a64bbf5f45 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 15:27:24 +0100 Subject: [PATCH 13/26] returndatacopy and bugfix. --- libyul/optimiser/UnusedStoreEliminator.cpp | 58 +++++++++---------- libyul/optimiser/UnusedStoreEliminator.h | 7 ++- .../covering_calldatacopy_fixed.yul | 3 +- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index e2bbe7645a31..7e7d15cf7b2f 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -78,12 +78,10 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) ignoreMemory }; rse(_ast); - if ( - auto evmDialect = dynamic_cast(&_context.dialect); - evmDialect && evmDialect->providesObjectAccess() - ) - { - } + + auto evmDialect = dynamic_cast(&_context.dialect); + if (evmDialect && evmDialect->providesObjectAccess()) + rse.clearActive(Location::Memory); else rse.markActiveAsUsed(Location::Memory); rse.markActiveAsUsed(Location::Storage); @@ -168,25 +166,30 @@ void UnusedStoreEliminator::visit(Statement const& _statement) yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); if (isCandidateForRemoval) { - // TODO what is this special case? - //State initialState = State::Undecided; -// if (*instruction == Instruction::RETURNDATACOPY) -// { -// initialState = State::Used; -// auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); -// auto length = identifierNameIfSSA(funCall->arguments.at(2)); -// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); -// if (length && startOffset) -// { -// FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); -// if ( -// knowledge.knownToBeZero(*startOffset) && -// lengthCall && -// toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE -// ) -// initialState = State::Undecided; -// } -// } + if (*instruction == Instruction::RETURNDATACOPY) + { + // Out-of-bounds access to the returndata buffer results in a revert, + // so we are careful not to remove a potentially reverting call to a builtin. + // The only way the Solidity compiler uses `returndatacopy` is + // `returndatacopy(X, 0, returndatasize())`, so we only allow to remove this pattern + // (which is guaranteed to never cause an out-of-bounds revert). + bool allowReturndatacopyToBeRemoved = false; + auto startOffset = identifierNameIfSSA(funCall->arguments.at(1)); + auto length = identifierNameIfSSA(funCall->arguments.at(2)); + KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); + if (length && startOffset) + { + FunctionCall const* lengthCall = get_if(m_ssaValues.at(*length).value); + if ( + knowledge.knownToBeZero(*startOffset) && + lengthCall && + toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE + ) + allowReturndatacopyToBeRemoved = true; + } + if (!allowReturndatacopyToBeRemoved) + return; + } m_allStores.insert(&_statement); vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); @@ -198,11 +201,6 @@ void UnusedStoreEliminator::visit(Statement const& _statement) } } -void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) -{ - markActiveAsUsed(); -} - vector UnusedStoreEliminator::operationsFromFunctionCall( FunctionCall const& _functionCall ) const diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index 94620ab7b5bc..f8227bd1010b 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -98,14 +98,17 @@ class UnusedStoreEliminator: public UnusedStoreBase private: std::set& activeMemoryStores() { return m_activeStores["m"_yulstring]; } - std::set& activeStorageStores() { return m_activeStores["m"_yulstring]; } + std::set& activeStorageStores() { return m_activeStores["s"_yulstring]; } void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. markActiveAsUsed(); } - void finalizeFunctionDefinition(FunctionDefinition const&) override; + void finalizeFunctionDefinition(FunctionDefinition const&) override + { + markActiveAsUsed(); + } std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; void applyOperation(Operation const& _operation); diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul index 47e0b95fa3a3..bcafffcec271 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul @@ -39,8 +39,7 @@ // } // if calldataload(2) // { -// let _17 := 7 -// let _18 := 2 +// mstore8(2, 7) // calldatacopy(0, 0, 3) // } // if calldataload(3) From dc584fe2f1ba5041339a6e6a0eedce1eb606f8fb Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 15:51:10 +0100 Subject: [PATCH 14/26] improved loop --- libyul/optimiser/UnusedStoreEliminator.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 7e7d15cf7b2f..9fa15f2ca169 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -253,27 +253,29 @@ vector UnusedStoreEliminator::operationsFromFu void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) { - set toRemove; set& active = _operation.location == Location::Storage ? activeStorageStores() : activeMemoryStores(); - // TODO this loop could be done more efficiently - removing while iterating. - for (Statement const* statement: active) + auto it = active.begin(); + auto end = active.end(); + for (; it != end;) { + Statement const* statement = *it; Operation const& storeOperation = m_storeOperations.at(statement); if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) { // This store is read from, mark it as used and remove it from the active set. m_usedStores.insert(statement); - toRemove.insert(statement); + it = active.erase(it); } else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) // This store is overwritten before being read, remove it from the active set. - toRemove.insert(statement); + it = active.erase(it); + else + ++it; } - active -= toRemove; } bool UnusedStoreEliminator::knownUnrelated( From d89c5638f02092dc3a9edd75e323cdd4c51db8ce Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 16:30:32 +0100 Subject: [PATCH 15/26] more work --- libyul/optimiser/UnusedAssignEliminator.cpp | 44 ++++++++------------- libyul/optimiser/UnusedAssignEliminator.h | 2 - libyul/optimiser/UnusedStoreBase.cpp | 2 + libyul/optimiser/UnusedStoreBase.h | 9 +++-- libyul/optimiser/UnusedStoreEliminator.cpp | 11 +++--- 5 files changed, 30 insertions(+), 38 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index eaa076fed3d5..6bb9a0643357 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -45,17 +45,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) UnusedAssignEliminator rae{_context.dialect}; rae(_ast); - set toRemove; - for (Statement const* unusedStore: rae.m_allStores - rae.m_usedStores) - // TODO this should also use user function side effects. - // Then we have to modify the multi-assign test (or verify that it is fine after all - // by adding a test where one var is used but not the other) - if (SideEffectsCollector{_context.dialect, *std::get(*unusedStore).value}.movable()) - toRemove.insert(unusedStore); - else - cerr << "not used because not movable" << endl; + rae.m_storesToRemove += move(rae.m_potentiallyUnusedStores); - StatementRemover remover{toRemove}; + StatementRemover remover{rae.m_storesToRemove}; remover(_ast); } @@ -64,14 +56,6 @@ void UnusedAssignEliminator::operator()(Identifier const& _identifier) markUsed(_identifier.name); } -void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDeclaration) -{ - UnusedStoreBase::operator()(_variableDeclaration); - - for (auto const& var: _variableDeclaration.variables) - m_declaredVariables.emplace(var.name); -} - void UnusedAssignEliminator::operator()(Assignment const& _assignment) { visit(*_assignment.value); @@ -81,7 +65,6 @@ void UnusedAssignEliminator::operator()(Assignment const& _assignment) void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefinition) { - ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {}); ScopedSaveAndRestore outerReturnVariables(m_returnVariables, {}); for (auto const& retParam: _functionDefinition.returnVariables) @@ -98,10 +81,9 @@ void UnusedAssignEliminator::operator()(Leave const&) void UnusedAssignEliminator::operator()(Block const& _block) { - ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {}); - UnusedStoreBase::operator()(_block); + // TODO we could also move some statements from "potentially" to "toRemove". for (auto const& statement: _block.statements) if (auto const* varDecl = get_if(&statement)) for (auto const& var: varDecl->variables) @@ -114,9 +96,18 @@ void UnusedAssignEliminator::visit(Statement const& _statement) if (auto const* assignment = get_if(&_statement)) { - m_allStores.insert(&_statement); - for (auto const& var: assignment->variableNames) - m_activeStores[var.name] = {&_statement}; + // TODO this should also use user function side effects. + // Then we have to modify the multi-assign test (or verify that it is fine after all + // by adding a test where one var is used but not the other) + if (SideEffectsCollector{m_dialect, *assignment->value}.movable()) + { + m_potentiallyUnusedStores.insert(&_statement); + for (auto const& var: assignment->variableNames) + m_activeStores[var.name] = {&_statement}; + } + else + for (auto const& var: assignment->variableNames) + m_activeStores[var.name].clear(); } // cerr << "After " << std::visit(AsmPrinter{}, _statement) << endl; @@ -144,7 +135,7 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) auto zeroIt = _zeroRuns.find(variable); if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; - m_usedStores.insert(assignment); + m_potentiallyUnusedStores.erase(assignment); } } @@ -157,7 +148,6 @@ void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const void UnusedAssignEliminator::markUsed(YulString _variable) { for (auto& assignment: m_activeStores[_variable]) - m_usedStores.insert(assignment); - // TODO is this correct? + m_potentiallyUnusedStores.erase(assignment); m_activeStores.erase(_variable); } diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index d90278b77223..75d9659c47f9 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -116,7 +116,6 @@ class UnusedAssignEliminator: public UnusedStoreBase explicit UnusedAssignEliminator(Dialect const& _dialect): UnusedStoreBase(_dialect) {} void operator()(Identifier const& _identifier) override; - void operator()(VariableDeclaration const& _variableDeclaration) override; void operator()(Assignment const& _assignment) override; void operator()(FunctionDefinition const&) override; void operator()(Leave const&) override; @@ -131,7 +130,6 @@ class UnusedAssignEliminator: public UnusedStoreBase void markUsed(YulString _variable); - std::set m_declaredVariables; std::set m_returnVariables; }; diff --git a/libyul/optimiser/UnusedStoreBase.cpp b/libyul/optimiser/UnusedStoreBase.cpp index 2f1c0a5da023..2d70d74b74dc 100644 --- a/libyul/optimiser/UnusedStoreBase.cpp +++ b/libyul/optimiser/UnusedStoreBase.cpp @@ -74,10 +74,12 @@ void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition) ScopedSaveAndRestore outerAssignments(m_activeStores, {}); ScopedSaveAndRestore forLoopInfo(m_forLoopInfo, {}); ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, 0); + ScopedSaveAndRestore potentiallyUnused(m_potentiallyUnusedStores, {}); (*this)(_functionDefinition.body); finalizeFunctionDefinition(_functionDefinition); + m_storesToRemove += move(m_potentiallyUnusedStores); } void UnusedStoreBase::operator()(ForLoop const& _forLoop) diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 35989da014c1..563d28677938 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -74,10 +74,11 @@ class UnusedStoreBase: public ASTWalker static void merge(ActiveStores& _target, std::vector&& _source); Dialect const& m_dialect; - /// Set of all stores encountered during the traversal - std::set m_allStores; - /// Set of stores that are marked as being used. - std::set m_usedStores; + /// Set of stores that unused. Once a store is deemed used, it is removed from here. + std::set m_potentiallyUnusedStores; + /// Statements are moved from m_potentiallUnusedStores to m_storesToRemove at the + /// end of each function. + std::set m_storesToRemove; /// Active (undecided) stores in the current branch. ActiveStores m_activeStores; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 9fa15f2ca169..09e946f58a84 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -85,8 +85,9 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) else rse.markActiveAsUsed(Location::Memory); rse.markActiveAsUsed(Location::Storage); + rse.m_storesToRemove += move(rse.m_potentiallyUnusedStores); - StatementRemover remover{rse.m_allStores - rse.m_usedStores}; + StatementRemover remover{rse.m_storesToRemove}; remover(_ast); } @@ -190,7 +191,7 @@ void UnusedStoreEliminator::visit(Statement const& _statement) if (!allowReturndatacopyToBeRemoved) return; } - m_allStores.insert(&_statement); + m_potentiallyUnusedStores.insert(&_statement); vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); if (operations.front().location == Location::Storage) @@ -267,7 +268,7 @@ void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation cons if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) { // This store is read from, mark it as used and remove it from the active set. - m_usedStores.insert(statement); + m_potentiallyUnusedStores.erase(statement); it = active.erase(it); } else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) @@ -402,10 +403,10 @@ void UnusedStoreEliminator::markActiveAsUsed( { if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) for (Statement const* statement: activeMemoryStores()) - m_usedStores.insert(statement); + m_potentiallyUnusedStores.erase(statement); if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) for (Statement const* statement: activeStorageStores()) - m_usedStores.insert(statement); + m_potentiallyUnusedStores.erase(statement); clearActive(_onlyLocation); } From ddf1d023bdb7877b82fdf8f668c827f8714ad506 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 17:10:02 +0100 Subject: [PATCH 16/26] fix it again --- libyul/optimiser/UnusedAssignEliminator.cpp | 10 +++++----- libyul/optimiser/UnusedStoreBase.cpp | 5 +++-- libyul/optimiser/UnusedStoreBase.h | 11 ++++++----- libyul/optimiser/UnusedStoreEliminator.cpp | 12 ++++++------ 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 6bb9a0643357..55a4066e5f17 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -45,9 +45,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) UnusedAssignEliminator rae{_context.dialect}; rae(_ast); - rae.m_storesToRemove += move(rae.m_potentiallyUnusedStores); + rae.m_storesToRemove += rae.m_allStores - rae.m_usedStores; - StatementRemover remover{rae.m_storesToRemove}; + StatementRemover remover{std::set{rae.m_storesToRemove.begin(), rae.m_storesToRemove.end()}}; remover(_ast); } @@ -101,7 +101,7 @@ void UnusedAssignEliminator::visit(Statement const& _statement) // by adding a test where one var is used but not the other) if (SideEffectsCollector{m_dialect, *assignment->value}.movable()) { - m_potentiallyUnusedStores.insert(&_statement); + m_allStores.insert(&_statement); for (auto const& var: assignment->variableNames) m_activeStores[var.name] = {&_statement}; } @@ -135,7 +135,7 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) auto zeroIt = _zeroRuns.find(variable); if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; - m_potentiallyUnusedStores.erase(assignment); + m_usedStores.insert(assignment); } } @@ -148,6 +148,6 @@ void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const void UnusedAssignEliminator::markUsed(YulString _variable) { for (auto& assignment: m_activeStores[_variable]) - m_potentiallyUnusedStores.erase(assignment); + m_usedStores.insert(assignment); m_activeStores.erase(_variable); } diff --git a/libyul/optimiser/UnusedStoreBase.cpp b/libyul/optimiser/UnusedStoreBase.cpp index 2d70d74b74dc..4a5123b6f0ae 100644 --- a/libyul/optimiser/UnusedStoreBase.cpp +++ b/libyul/optimiser/UnusedStoreBase.cpp @@ -71,15 +71,16 @@ void UnusedStoreBase::operator()(Switch const& _switch) void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition) { + ScopedSaveAndRestore allStores(m_allStores, {}); + ScopedSaveAndRestore usedStoresStores(m_usedStores, {}); ScopedSaveAndRestore outerAssignments(m_activeStores, {}); ScopedSaveAndRestore forLoopInfo(m_forLoopInfo, {}); ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, 0); - ScopedSaveAndRestore potentiallyUnused(m_potentiallyUnusedStores, {}); (*this)(_functionDefinition.body); finalizeFunctionDefinition(_functionDefinition); - m_storesToRemove += move(m_potentiallyUnusedStores); + m_storesToRemove += m_allStores - m_usedStores; } void UnusedStoreBase::operator()(ForLoop const& _forLoop) diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 563d28677938..7568620f7195 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -74,11 +74,12 @@ class UnusedStoreBase: public ASTWalker static void merge(ActiveStores& _target, std::vector&& _source); Dialect const& m_dialect; - /// Set of stores that unused. Once a store is deemed used, it is removed from here. - std::set m_potentiallyUnusedStores; - /// Statements are moved from m_potentiallUnusedStores to m_storesToRemove at the - /// end of each function. - std::set m_storesToRemove; + /// Set of all stores encountered during the traversal (in the current function). + std::set m_allStores; + /// Set of stores that are marked as being used (in the current function). + std::set m_usedStores; + /// List of stores that can be removed (globally). + std::vector m_storesToRemove; /// Active (undecided) stores in the current branch. ActiveStores m_activeStores; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 09e946f58a84..2ba0ec3d0ed8 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -85,9 +85,9 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) else rse.markActiveAsUsed(Location::Memory); rse.markActiveAsUsed(Location::Storage); - rse.m_storesToRemove += move(rse.m_potentiallyUnusedStores); + rse.m_storesToRemove += rse.m_allStores - rse.m_usedStores; - StatementRemover remover{rse.m_storesToRemove}; + StatementRemover remover{std::set{rse.m_storesToRemove.begin(), rse.m_storesToRemove.end()}}; remover(_ast); } @@ -191,7 +191,7 @@ void UnusedStoreEliminator::visit(Statement const& _statement) if (!allowReturndatacopyToBeRemoved) return; } - m_potentiallyUnusedStores.insert(&_statement); + m_allStores.insert(&_statement); vector operations = operationsFromFunctionCall(*funCall); yulAssert(operations.size() == 1, ""); if (operations.front().location == Location::Storage) @@ -268,7 +268,7 @@ void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation cons if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) { // This store is read from, mark it as used and remove it from the active set. - m_potentiallyUnusedStores.erase(statement); + m_usedStores.insert(statement); it = active.erase(it); } else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) @@ -403,10 +403,10 @@ void UnusedStoreEliminator::markActiveAsUsed( { if (_onlyLocation == nullopt || _onlyLocation == Location::Memory) for (Statement const* statement: activeMemoryStores()) - m_potentiallyUnusedStores.erase(statement); + m_usedStores.insert(statement); if (_onlyLocation == nullopt || _onlyLocation == Location::Storage) for (Statement const* statement: activeStorageStores()) - m_potentiallyUnusedStores.erase(statement); + m_usedStores.insert(statement); clearActive(_onlyLocation); } From 9ce7aa77d43d18ed94f08ab396bc8dfffde7d0e5 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 17:11:29 +0100 Subject: [PATCH 17/26] update expectations. --- .../unusedStoreEliminator/covering_calldatacopy_fixed.yul | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul index bcafffcec271..47e0b95fa3a3 100644 --- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul +++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul @@ -39,7 +39,8 @@ // } // if calldataload(2) // { -// mstore8(2, 7) +// let _17 := 7 +// let _18 := 2 // calldatacopy(0, 0, 3) // } // if calldataload(3) From f3bc7157df70e72cc314df35696a4c8050925dfb Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 28 Nov 2022 17:17:16 +0100 Subject: [PATCH 18/26] fix --- libyul/optimiser/UnusedAssignEliminator.cpp | 3 ++- libyul/optimiser/UnusedStoreEliminator.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 55a4066e5f17..ae92157c5eac 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -47,7 +47,8 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) rae.m_storesToRemove += rae.m_allStores - rae.m_usedStores; - StatementRemover remover{std::set{rae.m_storesToRemove.begin(), rae.m_storesToRemove.end()}}; + set toRemove{rae.m_storesToRemove.begin(), rae.m_storesToRemove.end()}; + StatementRemover remover{toRemove}; remover(_ast); } diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index 2ba0ec3d0ed8..d544d25e2aaa 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -87,7 +87,8 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast) rse.markActiveAsUsed(Location::Storage); rse.m_storesToRemove += rse.m_allStores - rse.m_usedStores; - StatementRemover remover{std::set{rse.m_storesToRemove.begin(), rse.m_storesToRemove.end()}}; + set toRemove{rse.m_storesToRemove.begin(), rse.m_storesToRemove.end()}; + StatementRemover remover{toRemove}; remover(_ast); } From c7bb117d5080f068c194075fecf554b97721dd51 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Nov 2022 11:39:16 +0100 Subject: [PATCH 19/26] Fix spelling. --- libyul/optimiser/UnusedStoreEliminator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index f8227bd1010b..6e5dbadc3678 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -49,7 +49,7 @@ struct AssignedValue; * to sstore, as we don't know whether the memory location will be read once we leave the function's scope, * so the statement will be removed only if all code code paths lead to a memory overwrite. * - * The m_store member of UnusedStoreBase uses the key "m" for memory ond "s" for storage stores. + * The m_store member of UnusedStoreBase uses the key "m" for memory and "s" for storage stores. * * Best run in SSA form. * From 90a147d3710c8c689aa6be6dc01352f8477811fd Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Nov 2022 11:56:58 +0100 Subject: [PATCH 20/26] Update documentation. --- libyul/optimiser/UnusedAssignEliminator.h | 47 ++++++++++++----------- libyul/optimiser/UnusedStoreBase.h | 7 +++- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index 75d9659c47f9..afcc6f00e7fe 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -62,28 +62,34 @@ struct Dialect; * Detailed rules: * * The AST is traversed twice: in an information gathering step and in the - * actual removal step. During information gathering, we maintain a - * mapping from assignment statements to the three states - * "unused", "undecided" and "used". - * When an assignment is visited, it is added to the mapping in the "undecided" state - * (see remark about for loops below) and every other assignment to the same variable - * that is still in the "undecided" state is changed to "unused". - * When a variable is referenced, the state of any assignment to that variable still - * in the "undecided" state is changed to "used". - * At points where control flow splits, a copy - * of the mapping is handed over to each branch. At points where control flow - * joins, the two mappings coming from the two branches are combined in the following way: - * Statements that are only in one mapping or have the same state are used unchanged. - * Conflicting values are resolved in the following way: - * "unused", "undecided" -> "undecided" - * "unused", "used" -> "used" - * "undecided, "used" -> "used". + * actual removal step. During information gathering, assignment statements + * can be marked as "potentially unused" or as "used". + * + * When an assignment is visited, it is stored in the "set of all stores" and + * added to the branch-dependent "active" sets for the assigned variables. This active + * set for a variable contains all statements where that variable was last assigned to, i.e. + * where a read from that variable could read from. + * Furthermore, all other active sets for the assigned variables are cleared. + * + * When a reference to a variable is visited, the active assignments to that variable + * in the current branch are marked as "used". This mark is permanent. + * Also, the active set for this variable in the current branch is cleared. + * + * At points where control-flow splits, we maintain a copy of the active set + * (all other data structures are shared across branches). + * + * At control-flow joins, we combine the sets of active stores for each variable. + * + * In the example above, the active set right after the assignment "b := mload(a)" (but before + * the control-flow join) is "b := mload(a)"; the assignment "b := 2" was removed. + * After the control-flow join it will contain both "b := mload(a)" and "b := 2", coming from + * the two branches. * * For for-loops, the condition, body and post-part are visited twice, taking * the joining control-flow at the condition into account. * In other words, we create three control flow paths: Zero runs of the loop, * one run and two runs and then combine them at the end. - * Running at most twice is enough because there are only three different states. + * Running at most twice is enough because this takes into account all possible control-flow connections. * * Since this algorithm has exponential runtime in the nesting depth of for loops, * a shortcut is taken at a certain nesting level: We only use the zero- and @@ -95,12 +101,7 @@ struct Dialect; * * At ``leave`` statements, all return variables are set to "used". * - * When a variable goes out of scope, all statements still in the "undecided" - * state are changed to "unused", unless the variable is the return - * parameter of a function - there, the state changes to "used". - * - * In the second traversal, all assignments that are in the "unused" state are removed. - * + * In the second traversal, all assignments that are not marked as "used" are removed. * * This step is usually run right after the SSA transform to complete * the generation of the pseudo-SSA. diff --git a/libyul/optimiser/UnusedStoreBase.h b/libyul/optimiser/UnusedStoreBase.h index 7568620f7195..0d618d8d254f 100644 --- a/libyul/optimiser/UnusedStoreBase.h +++ b/libyul/optimiser/UnusedStoreBase.h @@ -38,8 +38,11 @@ struct Dialect; * * The class tracks the state of abstract "stores" (assignments or mstore/sstore * statements) across the control-flow. It is the job of the derived class to create - * the stores and track references, but the base class adjusts their "used state" at - * control-flow splits and joins. + * the stores and track references, but the base class manages control-flow splits and joins. + * + * In general, active stores are those where it has not yet been determined if they are used + * or not. Those are split and joined at control-flow forks. Once a store has been deemed + * used, it is removed from the active set and marked as used and this will never change. * * Prerequisite: Disambiguator, ForLoopInitRewriter. */ From bc127bcc6d3f91a606d9ab4eaf6b699436708d18 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Nov 2022 12:15:49 +0100 Subject: [PATCH 21/26] Take non-continuing control-flow into account. --- libyul/optimiser/UnusedAssignEliminator.cpp | 23 ++++++++++++- libyul/optimiser/UnusedAssignEliminator.h | 17 ++++++++-- ...nally_assign_before_conditional_revert.yul | 33 +++++++++++++++++++ .../conditionally_assign_before_leave.yul | 22 +++++++++++++ .../conditionally_assign_before_revert.yul | 6 +--- 5 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul create mode 100644 test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index ae92157c5eac..e5582c72c9cb 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -42,7 +43,10 @@ using namespace solidity::yul; void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) { - UnusedAssignEliminator rae{_context.dialect}; + UnusedAssignEliminator rae{ + _context.dialect, + ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed() + }; rae(_ast); rae.m_storesToRemove += rae.m_allStores - rae.m_usedStores; @@ -74,10 +78,27 @@ void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefin UnusedStoreBase::operator()(_functionDefinition); } +void UnusedAssignEliminator::operator()(FunctionCall const& _functionCall) +{ + UnusedStoreBase::operator()(_functionCall); + + ControlFlowSideEffects sideEffects; + if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) + sideEffects = builtin->controlFlowSideEffects; + else + sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); + + if (!sideEffects.canContinue) + // We do not return from the current function, so it is OK to also + // clear the return variables. + m_activeStores.clear(); +} + void UnusedAssignEliminator::operator()(Leave const&) { for (YulString name: m_returnVariables) markUsed(name); + m_activeStores.clear(); } void UnusedAssignEliminator::operator()(Block const& _block) diff --git a/libyul/optimiser/UnusedAssignEliminator.h b/libyul/optimiser/UnusedAssignEliminator.h index afcc6f00e7fe..800d626eb840 100644 --- a/libyul/optimiser/UnusedAssignEliminator.h +++ b/libyul/optimiser/UnusedAssignEliminator.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -99,7 +100,11 @@ struct Dialect; * For switch statements that have a "default"-case, there is no control-flow * part that skips the switch. * - * At ``leave`` statements, all return variables are set to "used". + * At ``leave`` statements, all return variables are set to "used" and the set of active statements + * is cleared. + * + * If a function or builtin is called that does not continue, the set of active statements is + * cleared for all variables. * * In the second traversal, all assignments that are not marked as "used" are removed. * @@ -114,11 +119,18 @@ class UnusedAssignEliminator: public UnusedStoreBase static constexpr char const* name{"UnusedAssignEliminator"}; static void run(OptimiserStepContext&, Block& _ast); - explicit UnusedAssignEliminator(Dialect const& _dialect): UnusedStoreBase(_dialect) {} + explicit UnusedAssignEliminator( + Dialect const& _dialect, + std::map _controlFlowSideEffects + ): + UnusedStoreBase(_dialect), + m_controlFlowSideEffects(_controlFlowSideEffects) + {} void operator()(Identifier const& _identifier) override; void operator()(Assignment const& _assignment) override; void operator()(FunctionDefinition const&) override; + void operator()(FunctionCall const& _functionCall) override; void operator()(Leave const&) override; void operator()(Block const& _block) override; @@ -132,6 +144,7 @@ class UnusedAssignEliminator: public UnusedStoreBase void markUsed(YulString _variable); std::set m_returnVariables; + std::map m_controlFlowSideEffects; }; } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul new file mode 100644 index 000000000000..3eeea6f4dd90 --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_conditional_revert.yul @@ -0,0 +1,33 @@ +{ + function g() { + if calldataload(10) { revert(0, 0) } + } + function f() { + let a := calldataload(0) + if calldataload(1) { + // this can NOT be removed + a := 2 + g() + } + sstore(0, a) + } +} +// ---- +// step: unusedAssignEliminator +// +// { +// function g() +// { +// if calldataload(10) { revert(0, 0) } +// } +// function f() +// { +// let a := calldataload(0) +// if calldataload(1) +// { +// a := 2 +// g() +// } +// sstore(0, a) +// } +// } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul new file mode 100644 index 000000000000..4564f5d07524 --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_leave.yul @@ -0,0 +1,22 @@ +{ + function f() { + let a := calldataload(0) + if calldataload(1) { + // this can be removed + a := 2 + leave + } + sstore(0, a) + } +} +// ---- +// step: unusedAssignEliminator +// +// { +// function f() +// { +// let a := calldataload(0) +// if calldataload(1) { leave } +// sstore(0, a) +// } +// } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul index 88b12264ef0a..469da01fc46f 100644 --- a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul @@ -12,10 +12,6 @@ // // { // let a := calldataload(0) -// if calldataload(1) -// { -// a := 2 -// revert(0, 0) -// } +// if calldataload(1) { revert(0, 0) } // sstore(0, a) // } From 7673d0cb74c65a16a6b084ed4ee50182656dcfa1 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 29 Nov 2022 12:18:57 +0100 Subject: [PATCH 22/26] Gas updates. --- .../semanticTests/externalContracts/deposit_contract.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/libsolidity/semanticTests/externalContracts/deposit_contract.sol b/test/libsolidity/semanticTests/externalContracts/deposit_contract.sol index 389661fd7a06..48f7247a66fe 100644 --- a/test/libsolidity/semanticTests/externalContracts/deposit_contract.sol +++ b/test/libsolidity/semanticTests/externalContracts/deposit_contract.sol @@ -176,7 +176,7 @@ contract DepositContract is IDepositContract, ERC165 { } // ---- // constructor() -// gas irOptimized: 1430741 +// gas irOptimized: 1419712 // gas legacy: 2427905 // gas legacyOptimized: 1773081 // supportsInterface(bytes4): 0x0 -> 0 From 984258dc64473778865ab833e1c23e5e3bd14b85 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 19 Dec 2022 14:33:57 +0100 Subject: [PATCH 23/26] Remove outdated comments. --- libyul/optimiser/UnusedAssignEliminator.cpp | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index e5582c72c9cb..0b784b562215 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -38,9 +38,6 @@ using namespace std; using namespace solidity; using namespace solidity::yul; -// TODO this component does not handle reverting function calls specially. Is that OK? -// We should set m_activeStores to empty set for a reverting function call, like wo do with `leave`. - void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) { UnusedAssignEliminator rae{ @@ -105,7 +102,6 @@ void UnusedAssignEliminator::operator()(Block const& _block) { UnusedStoreBase::operator()(_block); - // TODO we could also move some statements from "potentially" to "toRemove". for (auto const& statement: _block.statements) if (auto const* varDecl = get_if(&statement)) for (auto const& var: varDecl->variables) @@ -118,9 +114,6 @@ void UnusedAssignEliminator::visit(Statement const& _statement) if (auto const* assignment = get_if(&_statement)) { - // TODO this should also use user function side effects. - // Then we have to modify the multi-assign test (or verify that it is fine after all - // by adding a test where one var is used but not the other) if (SideEffectsCollector{m_dialect, *assignment->value}.movable()) { m_allStores.insert(&_statement); @@ -131,14 +124,6 @@ void UnusedAssignEliminator::visit(Statement const& _statement) for (auto const& var: assignment->variableNames) m_activeStores[var.name].clear(); } - -// cerr << "After " << std::visit(AsmPrinter{}, _statement) << endl; -// for (auto&& [var, assigns]: m_activeStores) -// { -// cerr << " " << var.str() << ":" << endl; -// for (auto const& assign: assigns) -// cerr << " " << std::visit(AsmPrinter{}, *assign) << endl; -// } } void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) @@ -147,9 +132,6 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) // Change all assignments that were newly introduced in the for loop to "used". // We do not have to do that with the "break" or "continue" paths, because // they will be joined later anyway. - // TODO parallel traversal might be more efficient here. - - // TODO is this correct? for (auto& [variable, stores]: m_activeStores) for (auto& assignment: stores) From 11e7c5e8138a404715c7efb7eedf293c4d5bac3f Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 19 Dec 2022 14:34:49 +0100 Subject: [PATCH 24/26] Rename local variable. --- libyul/optimiser/UnusedAssignEliminator.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 0b784b562215..d165d2bd31ae 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -40,15 +40,15 @@ using namespace solidity::yul; void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) { - UnusedAssignEliminator rae{ + UnusedAssignEliminator uae{ _context.dialect, ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed() }; - rae(_ast); + uae(_ast); - rae.m_storesToRemove += rae.m_allStores - rae.m_usedStores; + uae.m_storesToRemove += uae.m_allStores - uae.m_usedStores; - set toRemove{rae.m_storesToRemove.begin(), rae.m_storesToRemove.end()}; + set toRemove{uae.m_storesToRemove.begin(), uae.m_storesToRemove.end()}; StatementRemover remover{toRemove}; remover(_ast); } From c85a1058f6233cf8961de87e766102cfd9027ce3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 19 Dec 2022 15:00:24 +0100 Subject: [PATCH 25/26] Pull out the variable query. --- libyul/optimiser/UnusedAssignEliminator.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index d165d2bd31ae..c5cbc26c1b38 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -134,13 +134,15 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns) // they will be joined later anyway. for (auto& [variable, stores]: m_activeStores) + { + auto zeroIt = _zeroRuns.find(variable); for (auto& assignment: stores) { - auto zeroIt = _zeroRuns.find(variable); if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment)) continue; m_usedStores.insert(assignment); } + } } void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const& _functionDefinition) From 0bb233f1c0048074fc6e05973f68f260e2befea6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 20 Dec 2022 14:58:35 +0100 Subject: [PATCH 26/26] implement inves --- libyul/optimiser/DataFlowAnalyzer.cpp | 19 ++++++++++++++++--- libyul/optimiser/DataFlowAnalyzer.h | 4 +++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index 81028e30078c..32cbe121711f 100644 --- a/libyul/optimiser/DataFlowAnalyzer.cpp +++ b/libyul/optimiser/DataFlowAnalyzer.cpp @@ -271,7 +271,17 @@ void DataFlowAnalyzer::handleAssignment(set const& _variables, Expres auto const& referencedVariables = movableChecker.referencedVariables(); for (auto const& name: _variables) { + // TODO these might be interdependent and it could matter which order we + // run this loop! + if (!_isDeclaration) + { + for (YulString v: m_state.references[name]) + m_state.referencedBy[v].erase(name); + } + for (YulString v: referencedVariables) + m_state.referencedBy[v].insert(name); m_state.references[name] = referencedVariables; + if (!_isDeclaration) { // assignment to slot denoted by "name" @@ -316,6 +326,8 @@ void DataFlowAnalyzer::popScope() for (auto const& name: m_variableScopes.back().variables) { m_state.value.erase(name); + for (YulString v: m_state.references[name]) + m_state.referencedBy[v].erase(name); m_state.references.erase(name); } m_variableScopes.pop_back(); @@ -351,16 +363,17 @@ void DataFlowAnalyzer::clearValues(set _variables) _variables.count(_item.second); }); + // Use referencedBy // Also clear variables that reference variables to be cleared. for (auto const& variableToClear: _variables) - for (auto const& [ref, names]: m_state.references) - if (names.count(variableToClear)) - _variables.emplace(ref); + _variables += m_state.referencedBy[variableToClear]; // Clear the value and update the reference relation. for (auto const& name: _variables) { m_state.value.erase(name); + for (YulString v: m_state.references[name]) + m_state.referencedBy[v].erase(name); m_state.references.erase(name); } } diff --git a/libyul/optimiser/DataFlowAnalyzer.h b/libyul/optimiser/DataFlowAnalyzer.h index b449357e6606..e0a387cbce52 100644 --- a/libyul/optimiser/DataFlowAnalyzer.h +++ b/libyul/optimiser/DataFlowAnalyzer.h @@ -179,8 +179,10 @@ class DataFlowAnalyzer: public ASTModifier { /// Current values of variables, always movable. std::map value; - /// m_references[a].contains(b) <=> the current expression assigned to a references b + /// references[a].contains(b) <=> the current expression assigned to a references b std::unordered_map> references; + /// The inverse of references. + std::unordered_map> referencedBy; Environment environment; };