From 625d402dbbb8180240228d187f11e43fde244d98 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Wed, 2 Dec 2020 00:02:44 +0100 Subject: [PATCH] Various optimizations for the DataFlowAnalyzer. --- libsolutil/InvertibleMap.h | 62 +++++----------- libyul/YulString.h | 10 +++ .../CommonSubexpressionEliminator.cpp | 12 +-- libyul/optimiser/DataFlowAnalyzer.cpp | 73 +++++++++---------- libyul/optimiser/DataFlowAnalyzer.h | 5 +- libyul/optimiser/LoadResolver.cpp | 19 ++--- libyul/optimiser/Rematerialiser.cpp | 18 +++-- libyul/optimiser/StackCompressor.cpp | 2 +- 8 files changed, 89 insertions(+), 112 deletions(-) diff --git a/libsolutil/InvertibleMap.h b/libsolutil/InvertibleMap.h index 3c6f3238a..882890df5 100644 --- a/libsolutil/InvertibleMap.h +++ b/libsolutil/InvertibleMap.h @@ -18,8 +18,7 @@ #pragma once -#include -#include +#include /** * Data structure that keeps track of values and keys of a mapping. @@ -27,68 +26,41 @@ template struct InvertibleMap { - std::map values; - // references[x] == {y | values[y] == x} - std::map> references; + std::unordered_map values; void set(K _key, V _value) { - if (values.count(_key)) - references[values[_key]].erase(_key); values[_key] = _value; - references[_value].insert(_key); + } + + std::optional fetch(K _key) + { + auto it = values.find(_key); + if (it == values.end()) + return std::nullopt; + else + return it->second; } void eraseKey(K _key) { - if (values.count(_key)) - references[values[_key]].erase(_key); values.erase(_key); } void eraseValue(V _value) { - if (references.count(_value)) + auto it = values.begin(); + while (it != values.end()) { - for (V v: references[_value]) - values.erase(v); - references.erase(_value); + if (it->second == _value) + it = values.erase(it); + else + ++it; } } void clear() { values.clear(); - references.clear(); - } -}; - -template -struct InvertibleRelation -{ - /// forward[x] contains y <=> backward[y] contains x - std::map> forward; - std::map> backward; - - void insert(T _key, T _value) - { - forward[_key].insert(_value); - backward[_value].insert(_key); - } - - void set(T _key, std::set _values) - { - for (T v: forward[_key]) - backward[v].erase(_key); - for (T v: _values) - backward[v].insert(_key); - forward[_key] = std::move(_values); - } - - void eraseKey(T _key) - { - for (auto const& v: forward[_key]) - backward[v].erase(_key); - forward.erase(_key); } }; diff --git a/libyul/YulString.h b/libyul/YulString.h index 6985794ec..cee7bb9bf 100644 --- a/libyul/YulString.h +++ b/libyul/YulString.h @@ -168,3 +168,13 @@ inline YulString operator "" _yulstring(char const* _string, std::size_t _size) } } +namespace std +{ +template<> struct hash +{ + size_t operator()(solidity::yul::YulString const& x) const + { + return static_cast(x.hash()); + } +}; +} diff --git a/libyul/optimiser/CommonSubexpressionEliminator.cpp b/libyul/optimiser/CommonSubexpressionEliminator.cpp index e1c7ccced..148d56c8b 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.cpp +++ b/libyul/optimiser/CommonSubexpressionEliminator.cpp @@ -89,12 +89,9 @@ void CommonSubexpressionEliminator::visit(Expression& _e) if (m_value.count(name)) { assertThrow(m_value.at(name).value, OptimizerException, ""); - if (holds_alternative(*m_value.at(name).value)) - { - YulString value = std::get(*m_value.at(name).value).name; - assertThrow(inScope(value), OptimizerException, ""); - _e = Identifier{locationOf(_e), value}; - } + if (Identifier const* value = get_if(m_value.at(name).value)) + if (inScope(value->name)) + _e = Identifier{locationOf(_e), value->name}; } } else @@ -103,9 +100,8 @@ void CommonSubexpressionEliminator::visit(Expression& _e) for (auto const& [variable, value]: m_value) { assertThrow(value.value, OptimizerException, ""); - if (SyntacticallyEqual{}(_e, *value.value)) + if (SyntacticallyEqual{}(_e, *value.value) && inScope(variable)) { - assertThrow(inScope(variable), OptimizerException, ""); _e = Identifier{locationOf(_e), variable}; break; } diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index b845d2722..220a9d673 100644 --- a/libyul/optimiser/DataFlowAnalyzer.cpp +++ b/libyul/optimiser/DataFlowAnalyzer.cpp @@ -62,26 +62,26 @@ void DataFlowAnalyzer::operator()(ExpressionStatement& _statement) if (auto vars = isSimpleStore(StoreLoadLocation::Storage, _statement)) { ASTModifier::operator()(_statement); - set keysToErase; - for (auto const& item: m_storage.values) + auto it = m_storage.values.begin(); + while (it != m_storage.values.end()) if (!( - m_knowledgeBase.knownToBeDifferent(vars->first, item.first) || - m_knowledgeBase.knownToBeEqual(vars->second, item.second) + m_knowledgeBase.knownToBeDifferent(vars->first, it->first) || + m_knowledgeBase.knownToBeEqual(vars->second, it->second) )) - keysToErase.insert(item.first); - for (YulString const& key: keysToErase) - m_storage.eraseKey(key); + it = m_storage.values.erase(it); + else + ++it; m_storage.set(vars->first, vars->second); } else if (auto vars = isSimpleStore(StoreLoadLocation::Memory, _statement)) { ASTModifier::operator()(_statement); - set keysToErase; - for (auto const& item: m_memory.values) - if (!m_knowledgeBase.knownToBeDifferentByAtLeast32(vars->first, item.first)) - keysToErase.insert(item.first); - for (YulString const& key: keysToErase) - m_memory.eraseKey(key); + auto it = m_memory.values.begin(); + while (it != m_memory.values.end()) + if (!m_knowledgeBase.knownToBeDifferentByAtLeast32(vars->first, it->first)) + it = m_memory.values.erase(it); + else + ++it; m_memory.set(vars->first, vars->second); } else @@ -163,7 +163,7 @@ void DataFlowAnalyzer::operator()(FunctionDefinition& _fun) // but this could be difficult if it is subclassed. map value; size_t loopDepth{0}; - InvertibleRelation references; + unordered_map> references; InvertibleMap storage; InvertibleMap memory; swap(m_value, value); @@ -261,7 +261,7 @@ void DataFlowAnalyzer::handleAssignment(set const& _variables, Expres auto const& referencedVariables = movableChecker.referencedVariables(); for (auto const& name: _variables) { - m_references.set(name, referencedVariables); + m_references[name] = referencedVariables; if (!_isDeclaration) { // assignment to slot denoted by "name" @@ -298,7 +298,6 @@ void DataFlowAnalyzer::pushScope(bool _functionScope) void DataFlowAnalyzer::popScope() { - clearValues(std::move(m_variableScopes.back().variables)); m_variableScopes.pop_back(); } @@ -320,28 +319,28 @@ void DataFlowAnalyzer::clearValues(set _variables) // First clear storage knowledge, because we do not have to clear // storage knowledge of variables whose expression has changed, // since the value is still unchanged. - for (auto const& name: _variables) - { - // clear slot denoted by "name" - m_storage.eraseKey(name); - // clear slot contents denoted by "name" - m_storage.eraseValue(name); - // assignment to slot denoted by "name" - m_memory.eraseKey(name); - // assignment to slot contents denoted by "name" - m_memory.eraseValue(name); - } + auto clear = [&](auto&& values) { + auto it = values.begin(); + while (it != values.end()) + if (_variables.count(it->first) || _variables.count(it->second)) + it = values.erase(it); + else + ++it; + }; + clear(m_storage.values); + clear(m_memory.values); // Also clear variables that reference variables to be cleared. for (auto const& name: _variables) - for (auto const& ref: m_references.backward[name]) - _variables.emplace(ref); + for (auto const& [ref, names]: m_references) + if (names.count(name)) + _variables.emplace(ref); // Clear the value and update the reference relation. for (auto const& name: _variables) m_value.erase(name); for (auto const& name: _variables) - m_references.eraseKey(name); + m_references.erase(name); } void DataFlowAnalyzer::assignValue(YulString _variable, Expression const* _value) @@ -385,15 +384,15 @@ void DataFlowAnalyzer::joinKnowledgeHelper( // This also works for memory because _older is an "older version" // of m_memory and thus any overlapping write would have cleared the keys // that are not known to be different inside m_memory already. - set keysToErase; - for (auto const& item: _this.values) + auto it = _this.values.begin(); + while (it != _this.values.end()) { - auto it = _older.values.find(item.first); - if (it == _older.values.end() || it->second != item.second) - keysToErase.insert(item.first); + auto oldit = _older.values.find(it->first); + if (oldit != _older.values.end() && it->second == oldit->second) + ++it; + else + it = _this.values.erase(it); } - for (auto const& key: keysToErase) - _this.eraseKey(key); } bool DataFlowAnalyzer::inScope(YulString _variableName) const diff --git a/libyul/optimiser/DataFlowAnalyzer.h b/libyul/optimiser/DataFlowAnalyzer.h index 7a9b36a24..02b2cb261 100644 --- a/libyul/optimiser/DataFlowAnalyzer.h +++ b/libyul/optimiser/DataFlowAnalyzer.h @@ -163,9 +163,8 @@ protected: /// Current values of variables, always movable. std::map m_value; - /// m_references.forward[a].contains(b) <=> the current expression assigned to a references b - /// m_references.backward[b].contains(a) <=> the current expression assigned to a references b - InvertibleRelation m_references; + /// m_references[a].contains(b) <=> the current expression assigned to a references b + std::unordered_map> m_references; InvertibleMap m_storage; InvertibleMap m_memory; diff --git a/libyul/optimiser/LoadResolver.cpp b/libyul/optimiser/LoadResolver.cpp index 21992e547..d5e542216 100644 --- a/libyul/optimiser/LoadResolver.cpp +++ b/libyul/optimiser/LoadResolver.cpp @@ -65,15 +65,12 @@ void LoadResolver::tryResolve( return; YulString key = std::get(_arguments.at(0)).name; - if ( - _location == StoreLoadLocation::Storage && - m_storage.values.count(key) - ) - _e = Identifier{locationOf(_e), m_storage.values[key]}; - else if ( - m_optimizeMLoad && - _location == StoreLoadLocation::Memory && - m_memory.values.count(key) - ) - _e = Identifier{locationOf(_e), m_memory.values[key]}; + if (_location == StoreLoadLocation::Storage) + { + if (auto value = m_storage.fetch(key)) + _e = Identifier{locationOf(_e), *value}; + } + else if (m_optimizeMLoad && _location == StoreLoadLocation::Memory) + if (auto value = m_memory.fetch(key)) + _e = Identifier{locationOf(_e), *value}; } diff --git a/libyul/optimiser/Rematerialiser.cpp b/libyul/optimiser/Rematerialiser.cpp index abd1bcdd8..d32fd8ea6 100644 --- a/libyul/optimiser/Rematerialiser.cpp +++ b/libyul/optimiser/Rematerialiser.cpp @@ -86,13 +86,17 @@ void Rematerialiser::visit(Expression& _e) ) { assertThrow(m_referenceCounts[name] > 0, OptimizerException, ""); - for (auto const& ref: m_references.forward[name]) - assertThrow(inScope(ref), OptimizerException, ""); - // update reference counts - m_referenceCounts[name]--; - for (auto const& ref: ReferencesCounter::countReferences(*value.value)) - m_referenceCounts[ref.first] += ref.second; - _e = (ASTCopier{}).translate(*value.value); + bool allInScope = true; + for (auto const& ref: m_references[name]) + allInScope = allInScope && inScope(ref); + if (allInScope) + { + // update reference counts + m_referenceCounts[name]--; + for (auto const& ref: ReferencesCounter::countReferences(*value.value)) + m_referenceCounts[ref.first] += ref.second; + _e = (ASTCopier{}).translate(*value.value); + } } } } diff --git a/libyul/optimiser/StackCompressor.cpp b/libyul/optimiser/StackCompressor.cpp index a8dc9d871..fcd838387 100644 --- a/libyul/optimiser/StackCompressor.cpp +++ b/libyul/optimiser/StackCompressor.cpp @@ -57,7 +57,7 @@ public: for (auto const& codeCost: m_expressionCodeCost) { size_t numRef = m_numReferences[codeCost.first]; - cand.emplace(make_tuple(codeCost.second * numRef, codeCost.first, m_references.forward[codeCost.first])); + cand.emplace(make_tuple(codeCost.second * numRef, codeCost.first, m_references[codeCost.first])); } return cand; }