Merge pull request #10467 from ethereum/dataFlowAnalyzerOptimization

Various optimizations for the DataFlowAnalyzer.

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

libsolutil/CommonData.h

@@ -265,6 +265,39 @@ decltype(auto) valueOrDefault(MapType&& _map, KeyType const& _key, ValueType&& _
 	return (it == _map.end()) ? _defaultValue : it->second;
 }
 
+namespace detail
+{
+template<typename Callable>
+struct MapTuple
+{
+	Callable callable;
+	template<typename TupleType>
+	decltype(auto) operator()(TupleType&& _tuple) {
+		using PlainTupleType = std::remove_cv_t<std::remove_reference_t<TupleType>>;
+		return operator()(
+			std::forward<TupleType>(_tuple),
+			std::make_index_sequence<std::tuple_size_v<PlainTupleType>>{}
+		);
+	}
+private:
+	template<typename TupleType, size_t... I>
+	decltype(auto) operator()(TupleType&& _tuple, std::index_sequence<I...>)
+	{
+		return callable(std::get<I>(std::forward<TupleType>(_tuple))...);
+	}
+};
+}
+
+/// Wraps @a _callable, which takes multiple arguments, into a callable that takes a single tuple of arguments.
+/// Since structured binding in lambdas is not allowed, i.e. [](auto&& [key, value]) { ... } is invalid, this allows
+/// to instead use mapTuple([](auto&& key, auto&& value) { ... }).
+template<typename Callable>
+decltype(auto) mapTuple(Callable&& _callable)
+{
+	return detail::MapTuple<Callable>{std::forward<Callable>(_callable)};
+}
+
+
 // String conversion functions, mainly to/from hex/nibble/byte representations.
 
 enum class WhenError

libsolutil/InvertibleMap.h

@@ -1,94 +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 <http://www.gnu.org/licenses/>.
-*/
-// SPDX-License-Identifier: GPL-3.0
-
-#pragma once
-
-#include <map>
-#include <set>
-
-/**
- * Data structure that keeps track of values and keys of a mapping.
- */
-template <class K, class V>
-struct InvertibleMap
-{
-	std::map<K, V> values;
-	// references[x] == {y | values[y] == x}
-	std::map<V, std::set<K>> references;
-
-	void set(K _key, V _value)
-	{
-		if (values.count(_key))
-			references[values[_key]].erase(_key);
-		values[_key] = _value;
-		references[_value].insert(_key);
-	}
-
-	void eraseKey(K _key)
-	{
-		if (values.count(_key))
-			references[values[_key]].erase(_key);
-		values.erase(_key);
-	}
-
-	void eraseValue(V _value)
-	{
-		if (references.count(_value))
-		{
-			for (V v: references[_value])
-				values.erase(v);
-			references.erase(_value);
-		}
-	}
-
-	void clear()
-	{
-		values.clear();
-		references.clear();
-	}
-};
-
-template <class T>
-struct InvertibleRelation
-{
-	/// forward[x] contains y <=> backward[y] contains x
-	std::map<T, std::set<T>> forward;
-	std::map<T, std::set<T>> backward;
-
-	void insert(T _key, T _value)
-	{
-		forward[_key].insert(_value);
-		backward[_value].insert(_key);
-	}
-
-	void set(T _key, std::set<T> _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);
-	}
-};

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 <http://www.gnu.org/licenses/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+#include <map>
+
+// 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<Key, T, Compare, Alloc>::size_type erase_if(std::map<Key,T,Compare,Alloc>& _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<class Key, class T, class Hash, class KeyEqual, class Alloc, class Pred>
+typename std::unordered_map<Key, T, Hash, KeyEqual, Alloc>::size_type
+erase_if(std::unordered_map<Key, T, Hash, KeyEqual, Alloc>& _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();
+}
+
+}

libyul/YulString.h

@@ -168,3 +168,13 @@ inline YulString operator "" _yulstring(char const* _string, std::size_t _size)
 }
 
 }
+namespace std
+{
+template<> struct hash<solidity::yul::YulString>
+{
+	size_t operator()(solidity::yul::YulString const& x) const
+	{
+		return static_cast<size_t>(x.hash());
+	}
+};
+}

libyul/optimiser/CallGraphGenerator.h

@@ -23,8 +23,6 @@
 
 #include <libyul/optimiser/ASTWalker.h>
 
-#include <libsolutil/InvertibleMap.h>
-
 #include <map>
 #include <optional>
 #include <set>

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<Identifier>(*m_value.at(name).value))
-			{
-				YulString value = std::get<Identifier>(*m_value.at(name).value).name;
-				assertThrow(inScope(value), OptimizerException, "");
-				_e = Identifier{locationOf(_e), value};
-			}
+			if (Identifier const* value = get_if<Identifier>(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;
 			}

libyul/optimiser/DataFlowAnalyzer.cpp

@@ -29,6 +29,7 @@
 #include <libyul/Exceptions.h>
 
 #include <libsolutil/CommonData.h>
+#include <libsolutil/cxx20.h>
 
 #include <boost/range/adaptor/reversed.hpp>
 #include <boost/range/algorithm_ext/erase.hpp>
@@ -62,27 +63,20 @@ void DataFlowAnalyzer::operator()(ExpressionStatement& _statement)
 	if (auto vars = isSimpleStore(StoreLoadLocation::Storage, _statement))
 	{
 		ASTModifier::operator()(_statement);
-		set<YulString> keysToErase;
-		for (auto const& item: m_storage.values)
-			if (!(
-				m_knowledgeBase.knownToBeDifferent(vars->first, item.first) ||
-				m_knowledgeBase.knownToBeEqual(vars->second, item.second)
-			))
-				keysToErase.insert(item.first);
-		for (YulString const& key: keysToErase)
-			m_storage.eraseKey(key);
-		m_storage.set(vars->first, vars->second);
+		cxx20::erase_if(m_storage, mapTuple([&](auto&& key, auto&& value) {
+			return
+				!m_knowledgeBase.knownToBeDifferent(vars->first, key) &&
+				!m_knowledgeBase.knownToBeEqual(vars->second, value);
+		}));
+		m_storage[vars->first] = vars->second;
 	}
 	else if (auto vars = isSimpleStore(StoreLoadLocation::Memory, _statement))
 	{
 		ASTModifier::operator()(_statement);
-		set<YulString> 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);
-		m_memory.set(vars->first, vars->second);
+		cxx20::erase_if(m_memory, mapTuple([&](auto&& key, auto&& /* value */) {
+			return !m_knowledgeBase.knownToBeDifferentByAtLeast32(vars->first, key);
+		}));
+		m_memory[vars->first] = vars->second;
 	}
 	else
 	{
@@ -121,8 +115,8 @@ void DataFlowAnalyzer::operator()(VariableDeclaration& _varDecl)
 void DataFlowAnalyzer::operator()(If& _if)
 {
 	clearKnowledgeIfInvalidated(*_if.condition);
-	InvertibleMap<YulString, YulString> storage = m_storage;
-	InvertibleMap<YulString, YulString> memory = m_memory;
+	unordered_map<YulString, YulString> storage = m_storage;
+	unordered_map<YulString, YulString> memory = m_memory;
 
 	ASTModifier::operator()(_if);
 
@@ -140,8 +134,8 @@ void DataFlowAnalyzer::operator()(Switch& _switch)
 	set<YulString> assignedVariables;
 	for (auto& _case: _switch.cases)
 	{
-		InvertibleMap<YulString, YulString> storage = m_storage;
-		InvertibleMap<YulString, YulString> memory = m_memory;
+		unordered_map<YulString, YulString> storage = m_storage;
+		unordered_map<YulString, YulString> memory = m_memory;
 		(*this)(_case.body);
 		joinKnowledge(storage, memory);
 
@@ -163,9 +157,9 @@ void DataFlowAnalyzer::operator()(FunctionDefinition& _fun)
 	// but this could be difficult if it is subclassed.
 	map<YulString, AssignedValue> value;
 	size_t loopDepth{0};
-	InvertibleRelation<YulString> references;
-	InvertibleMap<YulString, YulString> storage;
-	InvertibleMap<YulString, YulString> memory;
+	unordered_map<YulString, set<YulString>> references;
+	unordered_map<YulString, YulString> storage;
+	unordered_map<YulString, YulString> memory;
 	swap(m_value, value);
 	swap(m_loopDepth, loopDepth);
 	swap(m_references, references);
@@ -261,17 +255,17 @@ void DataFlowAnalyzer::handleAssignment(set<YulString> 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"
-			m_storage.eraseKey(name);
+			m_storage.erase(name);
 			// assignment to slot contents denoted by "name"
-			m_storage.eraseValue(name);
+			cxx20::erase_if(m_storage, mapTuple([&name](auto&& /* key */, auto&& value) { return value == 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, mapTuple([&name](auto&& /* key */, auto&& value) { return value == name; }));
 		}
 	}
 
@@ -284,9 +278,9 @@ void DataFlowAnalyzer::handleAssignment(set<YulString> 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;
 		}
 	}
 }
@@ -298,7 +292,11 @@ void DataFlowAnalyzer::pushScope(bool _functionScope)
 
 void DataFlowAnalyzer::popScope()
 {
-	clearValues(std::move(m_variableScopes.back().variables));
+	for (auto const& name: m_variableScopes.back().variables)
+	{
+		m_value.erase(name);
+		m_references.erase(name);
+	}
 	m_variableScopes.pop_back();
 }
 
@@ -320,28 +318,24 @@ void DataFlowAnalyzer::clearValues(set<YulString> _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 eraseCondition = mapTuple([&_variables](auto&& key, auto&& value) {
+		return _variables.count(key) || _variables.count(value);
+	});
+	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)
-		for (auto const& ref: m_references.backward[name])
-			_variables.emplace(ref);
+	for (auto const& variableToClear: _variables)
+		for (auto const& [ref, names]: m_references)
+			if (names.count(variableToClear))
+				_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)
@@ -368,8 +362,8 @@ void DataFlowAnalyzer::clearKnowledgeIfInvalidated(Expression const& _expr)
 }
 
 void DataFlowAnalyzer::joinKnowledge(
-	InvertibleMap<YulString, YulString> const& _olderStorage,
-	InvertibleMap<YulString, YulString> const& _olderMemory
+	unordered_map<YulString, YulString> const& _olderStorage,
+	unordered_map<YulString, YulString> const& _olderMemory
 )
 {
 	joinKnowledgeHelper(m_storage, _olderStorage);
@@ -377,23 +371,18 @@ void DataFlowAnalyzer::joinKnowledge(
 }
 
 void DataFlowAnalyzer::joinKnowledgeHelper(
-	InvertibleMap<YulString, YulString>& _this,
-	InvertibleMap<YulString, YulString> const& _older
+	std::unordered_map<YulString, YulString>& _this,
+	std::unordered_map<YulString, YulString> 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.
-	set<YulString> keysToErase;
-	for (auto const& item: _this.values)
-	{
-		auto it = _older.values.find(item.first);
-		if (it == _older.values.end() || it->second != item.second)
-			keysToErase.insert(item.first);
-	}
-	for (auto const& key: keysToErase)
-		_this.eraseKey(key);
+	cxx20::erase_if(_this, mapTuple([&_older](auto&& key, auto&& currentValue){
+		YulString const* oldValue = valueOrNullptr(_older, key);
+		return !oldValue || *oldValue != currentValue;
+	}));
 }
 
 bool DataFlowAnalyzer::inScope(YulString _variableName) const

libyul/optimiser/DataFlowAnalyzer.h

@@ -29,8 +29,6 @@
 #include <libyul/AST.h> // Needed for m_zero below.
 #include <libyul/SideEffects.h>
 
-#include <libsolutil/InvertibleMap.h>
-
 #include <map>
 #include <set>
 
@@ -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<YulString, YulString> const& _olderStorage,
-		InvertibleMap<YulString, YulString> const& _olderMemory
+		std::unordered_map<YulString, YulString> const& _olderStorage,
+		std::unordered_map<YulString, YulString> const& _olderMemory
 	);
 
 	static void joinKnowledgeHelper(
-		InvertibleMap<YulString, YulString>& _thisData,
-		InvertibleMap<YulString, YulString> const& _olderData
+		std::unordered_map<YulString, YulString>& _thisData,
+		std::unordered_map<YulString, YulString> const& _olderData
 	);
 
 	/// Returns true iff the variable is in scope.
@@ -163,12 +161,11 @@ protected:
 
 	/// Current values of variables, always movable.
 	std::map<YulString, AssignedValue> 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<YulString> m_references;
+	/// m_references[a].contains(b) <=> the current expression assigned to a references b
+	std::unordered_map<YulString, std::set<YulString>> m_references;
 
-	InvertibleMap<YulString, YulString> m_storage;
-	InvertibleMap<YulString, YulString> m_memory;
+	std::unordered_map<YulString, YulString> m_storage;
+	std::unordered_map<YulString, YulString> m_memory;
 
 	KnowledgeBase m_knowledgeBase;
 

libyul/optimiser/LoadResolver.cpp

@@ -65,15 +65,14 @@ void LoadResolver::tryResolve(
 		return;
 
 	YulString key = std::get<Identifier>(_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 = util::valueOrNullptr(m_storage, key))
+			if (inScope(*value))
+				_e = Identifier{locationOf(_e), *value};
+	}
+	else if (m_optimizeMLoad && _location == StoreLoadLocation::Memory)
+		if (auto value = util::valueOrNullptr(m_memory, key))
+			if (inScope(*value))
+				_e = Identifier{locationOf(_e), *value};
 }

libyul/optimiser/Rematerialiser.cpp

@@ -26,6 +26,8 @@
 #include <libyul/Exceptions.h>
 #include <libyul/AST.h>
 
+#include <range/v3/algorithm/all_of.hpp>
+
 using namespace std;
 using namespace solidity;
 using namespace solidity::yul;
@@ -86,13 +88,14 @@ 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);
+				if (ranges::all_of(m_references[name], [&](auto const& ref) { return inScope(ref); }))
+				{
+					// 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);
+				}
 			}
 		}
 	}

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;
 	}