Avoid sorting FunctionDefinitions by AST ID during codegen

libsolidity/codegen/ir/IRGenerationContext.cpp

@@ -31,6 +31,7 @@
 #include <libsolutil/StringUtils.h>
 
 #include <range/v3/view/map.hpp>
+#include <range/v3/algorithm/find.hpp>
 
 using namespace solidity;
 using namespace solidity::util;
@@ -41,7 +42,7 @@ std::string IRGenerationContext::enqueueFunctionForCodeGeneration(FunctionDefini
 	std::string name = IRNames::function(_function);
 
 	if (!m_functions.contains(name))
-		m_functionGenerationQueue.insert(&_function);
+		m_functionGenerationQueue.push_back(&_function);
 
 	return name;
 }
@@ -50,8 +51,8 @@ FunctionDefinition const* IRGenerationContext::dequeueFunctionForCodeGeneration(
 {
 	solAssert(!m_functionGenerationQueue.empty(), "");
 
-	FunctionDefinition const* result = *m_functionGenerationQueue.begin();
-	m_functionGenerationQueue.erase(m_functionGenerationQueue.begin());
+	FunctionDefinition const* result = m_functionGenerationQueue.front();
+	m_functionGenerationQueue.pop_front();
 	return result;
 }
 
@@ -132,7 +133,7 @@ void IRGenerationContext::initializeInternalDispatch(InternalDispatchMap _intern
 {
 	solAssert(internalDispatchClean(), "");
 
-	for (DispatchSet const& functions: _internalDispatch | ranges::views::values)
+	for (DispatchQueue const& functions: _internalDispatch | ranges::views::values)
 		for (auto function: functions)
 			enqueueFunctionForCodeGeneration(*function);
 
@@ -152,13 +153,13 @@ void IRGenerationContext::addToInternalDispatch(FunctionDefinition const& _funct
 	solAssert(functionType, "");
 
 	YulArity arity = YulArity::fromType(*functionType);
-
-	if (m_internalDispatchMap.count(arity) != 0 && m_internalDispatchMap[arity].count(&_function) != 0)
-		// Note that m_internalDispatchMap[arity] is a set with a custom comparator, which looks at function IDs not definitions
-		solAssert(*m_internalDispatchMap[arity].find(&_function) == &_function, "Different definitions with the same function ID");
-
-	m_internalDispatchMap[arity].insert(&_function);
-	enqueueFunctionForCodeGeneration(_function);
+	auto it = ranges::find(m_internalDispatchMap[arity], &_function);
+	if (it != ranges::end(m_internalDispatchMap[arity]))
+		solAssert(*it == &_function, "Different definitions with the same function ID");
+	else {
+		m_internalDispatchMap[arity].push_back(&_function);
+		enqueueFunctionForCodeGeneration(_function);
+	}
 }
 
 

libsolidity/codegen/ir/IRGenerationContext.h

@@ -37,28 +37,15 @@
 #include <set>
 #include <string>
 #include <memory>
-#include <vector>
 
 namespace solidity::frontend
 {
 
 class YulUtilFunctions;
 class ABIFunctions;
 
-struct AscendingFunctionIDCompare
-{
-	bool operator()(FunctionDefinition const* _f1, FunctionDefinition const* _f2) const
-	{
-		// NULLs always first.
-		if (_f1 != nullptr && _f2 != nullptr)
-			return _f1->id() < _f2->id();
-		else
-			return _f1 == nullptr;
-	}
-};
-
-using DispatchSet = std::set<FunctionDefinition const*, AscendingFunctionIDCompare>;
-using InternalDispatchMap = std::map<YulArity, DispatchSet>;
+using DispatchQueue = std::deque<FunctionDefinition const*>;
+using InternalDispatchMap = std::map<YulArity, DispatchQueue>;
 
 /**
  * Class that contains contextual information during IR generation.
@@ -197,10 +184,9 @@ class IRGenerationContext
 	/// were discovered by the IR generator during AST traversal.
 	/// Note that the queue gets filled in a lazy way - new definitions can be added while the
 	/// collected ones get removed and traversed.
-	/// The order and duplicates are irrelevant here (hence std::set rather than std::queue) as
-	/// long as the order of Yul functions in the generated code is deterministic and the same on
-	/// all platforms - which is a property guaranteed by MultiUseYulFunctionCollector.
-	DispatchSet m_functionGenerationQueue;
+	/// The order and duplicates are relevant here
+	/// (see: IRGenerationContext::[enqueue|dequeue]FunctionForCodeGeneration)
+	DispatchQueue m_functionGenerationQueue;
 
 	/// Collection of functions that need to be callable via internal dispatch.
 	/// Note that having a key with an empty set of functions is a valid situation. It means that

test/libsolidity/semanticTests/abiEncoderV1/abi_encode_calldata_slice.sol

@@ -63,6 +63,6 @@ contract C {
 // gas legacy: 411269
 // gas legacyOptimized: 317754
 // test_uint256() ->
-// gas irOptimized: 509677
+// gas irOptimized: 509479
 // gas legacy: 577469
 // gas legacyOptimized: 441003

test/libsolidity/semanticTests/abiEncoderV2/abi_encode_calldata_slice.sol

@@ -64,6 +64,6 @@ contract C {
 // gas legacy: 411269
 // gas legacyOptimized: 317754
 // test_uint256() ->
-// gas irOptimized: 509677
+// gas irOptimized: 509479
 // gas legacy: 577469
 // gas legacyOptimized: 441003

test/libsolidity/semanticTests/externalContracts/base64.sol

@@ -33,7 +33,7 @@ contract test {
 // EVMVersion: >=constantinople
 // ----
 // constructor()
-// gas irOptimized: 406679
+// gas irOptimized: 406691
 // gas legacy: 737652
 // gas legacyOptimized: 527036
 // encode_inline_asm(bytes): 0x20, 0 -> 0x20, 0

test/libsolidity/semanticTests/externalContracts/prbmath_unsigned.sol

@@ -48,7 +48,7 @@ contract test {
 }
 // ----
 // constructor()
-// gas irOptimized: 1722598
+// gas irOptimized: 1722610
 // gas legacy: 2210160
 // gas legacyOptimized: 1734152
 // div(uint256,uint256): 3141592653589793238, 88714123 -> 35412542528203691288251815328

test/libsolidity/semanticTests/externalContracts/strings.sol

@@ -49,7 +49,7 @@ contract test {
 }
 // ----
 // constructor()
-// gas irOptimized: 634316
+// gas irOptimized: 634328
 // gas legacy: 1065857
 // gas legacyOptimized: 725423
 // toSlice(string): 0x20, 11, "hello world" -> 11, 0xa0