From ec8cd56c4fb13f081d92800fa9f74abe26cc4270 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 10 Nov 2022 11:49:40 +0100 Subject: [PATCH 1/9] Re-implement KnowledgeBase using groups of constantly-spaced variables. --- libyul/optimiser/DataFlowAnalyzer.cpp | 4 +- libyul/optimiser/KnowledgeBase.cpp | 148 +++++++++++++++------ libyul/optimiser/KnowledgeBase.h | 57 ++++++-- libyul/optimiser/UnusedStoreEliminator.cpp | 8 +- test/libyul/KnowledgeBaseTest.cpp | 8 +- 5 files changed, 167 insertions(+), 58 deletions(-) diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index 1d5519b57..11532a885 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 77848cce0..2fd5fb0a9 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); + 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 {}; +} + +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; + + if (value) + if (optional offset = explore(*value)) + return setOffset(_var, *offset); + return setOffset(_var, VariableOffset{_var, 0}); + +} + +optional KnowledgeBase::explore(Expression const& _value) +{ + 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}; + } + return {}; } -Expression KnowledgeBase::simplify(Expression _expression) +Expression const* KnowledgeBase::valueOf(YulString _var) { - m_counter = 0; - return simplifyRecursively(std::move(_expression)); + 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; } -Expression KnowledgeBase::simplifyRecursively(Expression _expression) +void KnowledgeBase::reset(YulString _var) { - if (m_counter++ > 100) - return _expression; - - if (holds_alternative(_expression)) - for (Expression& arg: std::get(_expression).arguments) - arg = simplifyRecursively(arg); - - if (auto match = SimplificationRules::findFirstMatch(_expression, m_dialect, m_variableValues)) - return simplifyRecursively(match->action().toExpression(debugDataOf(_expression))); - - return _expression; + 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 999d0e312..82c82a7e9 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 2e96be2e7..d5155ad7e 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 ec2f0313d..7cb5a8ae3 100644 --- a/test/libyul/KnowledgeBaseTest.cpp +++ b/test/libyul/KnowledgeBaseTest.cpp @@ -58,7 +58,7 @@ protected: 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 2/9] 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 d5155ad7e..245533395 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 8b5bfd7b1..7fbf9885f 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -68,13 +69,7 @@ public: 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 @@ private: 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 3/9] 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 2fd5fb0a9..23627653a 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 82c82a7e9..934b2e21a 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 @@ private: 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 245533395..3df9c6319 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 4/9] 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 23627653a..558dc8988 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 5/9] 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 1e8760250..409bb2983 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 6/9] 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 558dc8988..00d7cda1c 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 7/9] 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 00d7cda1c..36f07b74a 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 8/9] 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 36f07b74a..4b90acf9f 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 9/9] 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 4b90acf9f..96b352e40 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); } }