From 7fe03cbab0ac9a9610c4b477864026f90e332044 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Fri, 11 Dec 2020 17:30:06 +0100 Subject: [PATCH] Implement cxx20 polyfill and replace InvertibleMap entirely. --- libsolutil/CMakeLists.txt | 2 +- libsolutil/InvertibleMap.h | 66 -------------------- libsolutil/cxx20.h | 52 ++++++++++++++++ libyul/optimiser/CallGraphGenerator.h | 2 - libyul/optimiser/DataFlowAnalyzer.cpp | 90 +++++++++++---------------- libyul/optimiser/DataFlowAnalyzer.h | 14 ++--- libyul/optimiser/LoadResolver.cpp | 4 +- 7 files changed, 99 insertions(+), 131 deletions(-) delete mode 100644 libsolutil/InvertibleMap.h create mode 100644 libsolutil/cxx20.h diff --git a/libsolutil/CMakeLists.txt b/libsolutil/CMakeLists.txt index 7c10cf8b5..535b2b90f 100644 --- a/libsolutil/CMakeLists.txt +++ b/libsolutil/CMakeLists.txt @@ -8,6 +8,7 @@ set(sources CommonData.h CommonIO.cpp CommonIO.h + cxx20.h Exceptions.cpp Exceptions.h ErrorCodes.h @@ -15,7 +16,6 @@ set(sources FunctionSelector.h IndentedWriter.cpp IndentedWriter.h - InvertibleMap.h IpfsHash.cpp IpfsHash.h JSON.cpp diff --git a/libsolutil/InvertibleMap.h b/libsolutil/InvertibleMap.h deleted file mode 100644 index 882890df5..000000000 --- a/libsolutil/InvertibleMap.h +++ /dev/null @@ -1,66 +0,0 @@ -/* - This file is part of solidity. - - solidity is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - solidity is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with solidity. If not, see . -*/ -// SPDX-License-Identifier: GPL-3.0 - -#pragma once - -#include - -/** - * Data structure that keeps track of values and keys of a mapping. - */ -template -struct InvertibleMap -{ - std::unordered_map values; - - void set(K _key, V _value) - { - values[_key] = _value; - } - - 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) - { - values.erase(_key); - } - - void eraseValue(V _value) - { - auto it = values.begin(); - while (it != values.end()) - { - if (it->second == _value) - it = values.erase(it); - else - ++it; - } - } - - void clear() - { - values.clear(); - } -}; diff --git a/libsolutil/cxx20.h b/libsolutil/cxx20.h new file mode 100644 index 000000000..69e09a005 --- /dev/null +++ b/libsolutil/cxx20.h @@ -0,0 +1,52 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ +// SPDX-License-Identifier: GPL-3.0 + +#include + +// Contains polyfills of STL functions and algorithms that will become available in C++20. +namespace solidity::cxx20 +{ + +// Taken from https://en.cppreference.com/w/cpp/container/map/erase_if. +template< class Key, class T, class Compare, class Alloc, class Pred > +typename std::map::size_type erase_if(std::map& c, Pred pred) +{ + auto old_size = c.size(); + for (auto i = c.begin(), last = c.end(); i != last;) + if (pred(*i)) + i = c.erase(i); + else + ++i; + return old_size - c.size(); +} + +// Taken from https://en.cppreference.com/w/cpp/container/unordered_map/erase_if. +template +typename std::unordered_map::size_type +erase_if(std::unordered_map& c, Pred pred) +{ + auto old_size = c.size(); + for (auto i = c.begin(), last = c.end(); i != last;) + if (pred(*i)) + i = c.erase(i); + else + ++i; + return old_size - c.size(); +} + +} diff --git a/libyul/optimiser/CallGraphGenerator.h b/libyul/optimiser/CallGraphGenerator.h index 11224eeaa..a581df018 100644 --- a/libyul/optimiser/CallGraphGenerator.h +++ b/libyul/optimiser/CallGraphGenerator.h @@ -23,8 +23,6 @@ #include -#include - #include #include #include diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index 220a9d673..aee1e5e37 100644 --- a/libyul/optimiser/DataFlowAnalyzer.cpp +++ b/libyul/optimiser/DataFlowAnalyzer.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -62,27 +63,21 @@ void DataFlowAnalyzer::operator()(ExpressionStatement& _statement) if (auto vars = isSimpleStore(StoreLoadLocation::Storage, _statement)) { ASTModifier::operator()(_statement); - auto it = m_storage.values.begin(); - while (it != m_storage.values.end()) - if (!( - m_knowledgeBase.knownToBeDifferent(vars->first, it->first) || - m_knowledgeBase.knownToBeEqual(vars->second, it->second) - )) - it = m_storage.values.erase(it); - else - ++it; - m_storage.set(vars->first, vars->second); + cxx20::erase_if(m_storage, [&](auto const& entry) { + return !( + m_knowledgeBase.knownToBeDifferent(vars->first, entry.first) || + m_knowledgeBase.knownToBeEqual(vars->second, entry.second) + ); + }); + m_storage[vars->first] = vars->second; } else if (auto vars = isSimpleStore(StoreLoadLocation::Memory, _statement)) { ASTModifier::operator()(_statement); - 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); + cxx20::erase_if(m_memory, [&](auto const& entry) { + return !m_knowledgeBase.knownToBeDifferentByAtLeast32(vars->first, entry.first); + }); + m_memory[vars->first] = vars->second; } else { @@ -121,8 +116,8 @@ void DataFlowAnalyzer::operator()(VariableDeclaration& _varDecl) void DataFlowAnalyzer::operator()(If& _if) { clearKnowledgeIfInvalidated(*_if.condition); - InvertibleMap storage = m_storage; - InvertibleMap memory = m_memory; + unordered_map storage = m_storage; + unordered_map memory = m_memory; ASTModifier::operator()(_if); @@ -140,8 +135,8 @@ void DataFlowAnalyzer::operator()(Switch& _switch) set assignedVariables; for (auto& _case: _switch.cases) { - InvertibleMap storage = m_storage; - InvertibleMap memory = m_memory; + unordered_map storage = m_storage; + unordered_map memory = m_memory; (*this)(_case.body); joinKnowledge(storage, memory); @@ -164,8 +159,8 @@ void DataFlowAnalyzer::operator()(FunctionDefinition& _fun) map value; size_t loopDepth{0}; unordered_map> references; - InvertibleMap storage; - InvertibleMap memory; + unordered_map storage; + unordered_map memory; swap(m_value, value); swap(m_loopDepth, loopDepth); swap(m_references, references); @@ -265,13 +260,13 @@ void DataFlowAnalyzer::handleAssignment(set const& _variables, Expres if (!_isDeclaration) { // assignment to slot denoted by "name" - m_storage.eraseKey(name); + m_storage.erase(name); // assignment to slot contents denoted by "name" - m_storage.eraseValue(name); + cxx20::erase_if(m_storage, [&](auto const& entry) { return entry.second == name; }); // assignment to slot denoted by "name" - m_memory.eraseKey(name); + m_memory.erase(name); // assignment to slot contents denoted by "name" - m_memory.eraseValue(name); + cxx20::erase_if(m_memory, [&](auto const& entry) { return entry.second == name; }); } } @@ -284,9 +279,9 @@ void DataFlowAnalyzer::handleAssignment(set const& _variables, Expres // On the other hand, if we knew the value in the slot // already, then the sload() / mload() would have been replaced by a variable anyway. if (auto key = isSimpleLoad(StoreLoadLocation::Memory, *_value)) - m_memory.set(*key, variable); + m_memory[*key] = variable; else if (auto key = isSimpleLoad(StoreLoadLocation::Storage, *_value)) - m_storage.set(*key, variable); + m_storage[*key] = variable; } } } @@ -319,16 +314,11 @@ 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. - 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; + auto eraseCondition = [&](auto const& entry) { + return _variables.count(entry.first) || _variables.count(entry.second); }; - clear(m_storage.values); - clear(m_memory.values); + cxx20::erase_if(m_storage, eraseCondition); + cxx20::erase_if(m_memory, eraseCondition); // Also clear variables that reference variables to be cleared. for (auto const& name: _variables) @@ -338,9 +328,10 @@ void DataFlowAnalyzer::clearValues(set _variables) // Clear the value and update the reference relation. for (auto const& name: _variables) + { m_value.erase(name); - for (auto const& name: _variables) m_references.erase(name); + } } void DataFlowAnalyzer::assignValue(YulString _variable, Expression const* _value) @@ -367,8 +358,8 @@ void DataFlowAnalyzer::clearKnowledgeIfInvalidated(Expression const& _expr) } void DataFlowAnalyzer::joinKnowledge( - InvertibleMap const& _olderStorage, - InvertibleMap const& _olderMemory + unordered_map const& _olderStorage, + unordered_map const& _olderMemory ) { joinKnowledgeHelper(m_storage, _olderStorage); @@ -376,23 +367,18 @@ void DataFlowAnalyzer::joinKnowledge( } void DataFlowAnalyzer::joinKnowledgeHelper( - InvertibleMap& _this, - InvertibleMap const& _older + std::unordered_map& _this, + std::unordered_map const& _older ) { // We clear if the key does not exist in the older map or if the value is different. // 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. - auto it = _this.values.begin(); - while (it != _this.values.end()) - { - auto oldit = _older.values.find(it->first); - if (oldit != _older.values.end() && it->second == oldit->second) - ++it; - else - it = _this.values.erase(it); - } + cxx20::erase_if(_this, [&](auto const& entry){ + YulString const* value = valueOrNullptr(_older, entry.first); + return !value || *value != entry.second; + }); } bool DataFlowAnalyzer::inScope(YulString _variableName) const diff --git a/libyul/optimiser/DataFlowAnalyzer.h b/libyul/optimiser/DataFlowAnalyzer.h index 02b2cb261..7cbbe63b7 100644 --- a/libyul/optimiser/DataFlowAnalyzer.h +++ b/libyul/optimiser/DataFlowAnalyzer.h @@ -29,8 +29,6 @@ #include // Needed for m_zero below. #include -#include - #include #include @@ -124,13 +122,13 @@ protected: /// This only works if the current state is a direct successor of the older point, /// i.e. `_otherStorage` and `_otherMemory` cannot have additional changes. void joinKnowledge( - InvertibleMap const& _olderStorage, - InvertibleMap const& _olderMemory + std::unordered_map const& _olderStorage, + std::unordered_map const& _olderMemory ); static void joinKnowledgeHelper( - InvertibleMap& _thisData, - InvertibleMap const& _olderData + std::unordered_map& _thisData, + std::unordered_map const& _olderData ); /// Returns true iff the variable is in scope. @@ -166,8 +164,8 @@ protected: /// m_references[a].contains(b) <=> the current expression assigned to a references b std::unordered_map> m_references; - InvertibleMap m_storage; - InvertibleMap m_memory; + std::unordered_map m_storage; + std::unordered_map m_memory; KnowledgeBase m_knowledgeBase; diff --git a/libyul/optimiser/LoadResolver.cpp b/libyul/optimiser/LoadResolver.cpp index d5e542216..86e406115 100644 --- a/libyul/optimiser/LoadResolver.cpp +++ b/libyul/optimiser/LoadResolver.cpp @@ -67,10 +67,10 @@ void LoadResolver::tryResolve( YulString key = std::get(_arguments.at(0)).name; if (_location == StoreLoadLocation::Storage) { - if (auto value = m_storage.fetch(key)) + if (auto value = util::valueOrNullptr(m_storage, key)) _e = Identifier{locationOf(_e), *value}; } else if (m_optimizeMLoad && _location == StoreLoadLocation::Memory) - if (auto value = m_memory.fetch(key)) + if (auto value = util::valueOrNullptr(m_memory, key)) _e = Identifier{locationOf(_e), *value}; }