Merge pull request #12731 from ethereum/removeInliningConstraint

Remove inlining constraint for large functions when targetting the new code transform.

Author: chriseth

Changelog.md

@@ -7,6 +7,7 @@ Language Features:
 
 Compiler Features:
 * LSP: Add rudimentary support for semantic highlighting.
+ * Yul Optimizer: Improve inlining heuristics for via IR code generation and pure Yul compilation.
 
 
 Bugfixes:

libyul/optimiser/FullInliner.cpp

@@ -22,11 +22,14 @@
 #include <libyul/optimiser/FullInliner.h>
 
 #include <libyul/optimiser/ASTCopier.h>
+#include <libyul/optimiser/CallGraphGenerator.h>
+#include <libyul/optimiser/FunctionCallFinder.h>
 #include <libyul/optimiser/NameCollector.h>
 #include <libyul/optimiser/Metrics.h>
 #include <libyul/optimiser/SSAValueTracker.h>
 #include <libyul/optimiser/Semantics.h>
 #include <libyul/optimiser/CallGraphGenerator.h>
+#include <libyul/backends/evm/EVMDialect.h>
 #include <libyul/Exceptions.h>
 #include <libyul/AST.h>
 #include <libyul/Dialect.h>
@@ -46,8 +49,12 @@ void FullInliner::run(OptimiserStepContext& _context, Block& _ast)
 }
 
 FullInliner::FullInliner(Block& _ast, NameDispenser& _dispenser, Dialect const& _dialect):
-	m_ast(_ast), m_nameDispenser(_dispenser), m_dialect(_dialect)
+	m_ast(_ast),
+	m_recursiveFunctions(CallGraphGenerator::callGraph(_ast).recursiveFunctions()),
+	m_nameDispenser(_dispenser),
+	m_dialect(_dialect)
 {
+
 	// Determine constants
 	SSAValueTracker tracker;
 	tracker(m_ast);
@@ -71,6 +78,15 @@ FullInliner::FullInliner(Block& _ast, NameDispenser& _dispenser, Dialect const&
 			m_singleUse.emplace(fun.name);
 		updateCodeSize(fun);
 	}
+
+	// Check for memory guard.
+	vector<FunctionCall*> memoryGuardCalls = FunctionCallFinder::run(
+		_ast,
+		"memoryguard"_yulstring
+	);
+	// We will perform less aggressive inlining, if no ``memoryguard`` call is found.
+	if (!memoryGuardCalls.empty())
+		m_hasMemoryGuard = true;
 }
 
 void FullInliner::run(Pass _pass)
@@ -177,8 +193,20 @@ bool FullInliner::shallInline(FunctionCall const& _funCall, YulString _callSite)
 	if (m_pass == Pass::InlineTiny)
 		return false;
 
-	// Do not inline into already big functions.
-	if (m_functionSizes.at(_callSite) > 45)
+	bool aggressiveInlining = true;
+
+	if (
+		EVMDialect const* evmDialect = dynamic_cast<EVMDialect const*>(&m_dialect);
+		!evmDialect || !evmDialect->providesObjectAccess() || evmDialect->evmVersion() <= langutil::EVMVersion::homestead()
+	)
+		// No aggressive inlining with the old code transform.
+		aggressiveInlining = false;
+
+	// No aggressive inlining, if we cannot perform stack-to-memory.
+	if (!m_hasMemoryGuard || m_recursiveFunctions.count(_callSite))
+		aggressiveInlining = false;
+
+	if (!aggressiveInlining && m_functionSizes.at(_callSite) > 45)
 		return false;
 
 	if (m_singleUse.count(calledFunction->name))
@@ -196,7 +224,7 @@ bool FullInliner::shallInline(FunctionCall const& _funCall, YulString _callSite)
 			break;
 		}
 
-	return (size < 6 || (constantArg && size < 12));
+	return (size < (aggressiveInlining ? 8 : 6) || (constantArg && size < (aggressiveInlining ? 16 : 12)));
 }
 
 void FullInliner::tentativelyUpdateCodeSize(YulString _function, YulString _callSite)

libyul/optimiser/FullInliner.h

@@ -111,6 +111,10 @@ class FullInliner: public ASTModifier
 	std::map<YulString, FunctionDefinition*> m_functions;
 	/// Functions not to be inlined (because they contain the ``leave`` statement).
 	std::set<YulString> m_noInlineFunctions;
+	/// True, if the code contains a ``memoryguard`` and we can expect to be able to move variables to memory later.
+	bool m_hasMemoryGuard = false;
+	/// Set of recursive functions.
+	std::set<YulString> m_recursiveFunctions;
 	/// Names of functions to always inline.
 	std::set<YulString> m_singleUse;
 	/// Variables that are constants (used for inlining heuristic)

test/cmdlineTests/debug_info_in_yul_snippet_escaping/output

@@ -434,77 +434,77 @@ object "D_27" {
             {
                 /// @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..."
                 let _1 := memoryguard(0x80)
-                mstore(64, _1)
+                let _2 := 64
+                mstore(_2, _1)
                 if iszero(lt(calldatasize(), 4))
                 {
-                    let _2 := 0
-                    if eq(0x26121ff0, shr(224, calldataload(_2)))
+                    let _3 := 0
+                    if eq(0x26121ff0, shr(224, calldataload(_3)))
                     {
-                        if callvalue() { revert(_2, _2) }
-                        if slt(add(calldatasize(), not(3)), _2) { revert(_2, _2) }
+                        if callvalue() { revert(_3, _3) }
+                        if slt(add(calldatasize(), not(3)), _3) { revert(_3, _3) }
                         /// @src 0:446:491  "new /// @src 0:149:156  \"new C()\"..."
-                        let _3 := datasize("C_2")
-                        let _4 := add(_1, _3)
-                        if or(gt(_4, 0xffffffffffffffff), lt(_4, _1)) { panic_error_0x41() }
-                        datacopy(_1, dataoffset("C_2"), _3)
-                        if iszero(create(/** @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..." */ _2, /** @src 0:446:491  "new /// @src 0:149:156  \"new C()\"..." */ _1, sub(_4, _1)))
+                        let _4 := datasize("C_2")
+                        let _5 := add(_1, _4)
+                        let _6 := 0xffffffffffffffff
+                        if or(gt(_5, _6), lt(_5, _1))
                         {
                             /// @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..."
-                            let pos := mload(64)
-                            returndatacopy(pos, _2, returndatasize())
+                            mstore(_3, shl(224, 0x4e487b71))
+                            mstore(4, 0x41)
+                            revert(_3, 0x24)
+                        }
+                        /// @src 0:446:491  "new /// @src 0:149:156  \"new C()\"..."
+                        datacopy(_1, dataoffset("C_2"), _4)
+                        if iszero(create(/** @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..." */ _3, /** @src 0:446:491  "new /// @src 0:149:156  \"new C()\"..." */ _1, sub(_5, _1)))
+                        {
+                            /// @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..."
+                            let pos := mload(_2)
+                            returndatacopy(pos, _3, returndatasize())
                             revert(pos, returndatasize())
                         }
-                        mstore(add(allocate_memory_array_string(), 32), "/*")
-                        let memPtr := allocate_memory_array_string_546()
-                        mstore(add(memPtr, 32), 0x2f2a2a204073726320303a39363a313635202022636f6e74726163742044207b)
-                        mstore(add(memPtr, 64), shl(200, 0x2e2e2e22202a2f))
-                        let memPos := mload(64)
-                        return(memPos, sub(abi_encode_string(memPos, memPtr), memPos))
+                        let memPtr := mload(_2)
+                        let newFreePtr := add(memPtr, _2)
+                        if or(gt(newFreePtr, /** @src 0:446:491  "new /// @src 0:149:156  \"new C()\"..." */ _6), /** @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..." */ lt(newFreePtr, memPtr))
+                        {
+                            mstore(_3, shl(224, 0x4e487b71))
+                            mstore(4, 0x41)
+                            revert(_3, 0x24)
+                        }
+                        mstore(_2, newFreePtr)
+                        mstore(memPtr, 2)
+                        let _7 := 32
+                        mstore(add(memPtr, _7), "/*")
+                        let memPtr_1 := mload(_2)
+                        let newFreePtr_1 := add(memPtr_1, 96)
+                        if or(gt(newFreePtr_1, /** @src 0:446:491  "new /// @src 0:149:156  \"new C()\"..." */ _6), /** @src 0:279:599  "contract D /** @src 0:96:165  \"contract D {...\" *\/ {..." */ lt(newFreePtr_1, memPtr_1))
+                        {
+                            mstore(_3, shl(224, 0x4e487b71))
+                            mstore(4, 0x41)
+                            revert(_3, 0x24)
+                        }
+                        mstore(_2, newFreePtr_1)
+                        mstore(memPtr_1, 39)
+                        mstore(add(memPtr_1, _7), 0x2f2a2a204073726320303a39363a313635202022636f6e74726163742044207b)
+                        mstore(add(memPtr_1, _2), shl(200, 0x2e2e2e22202a2f))
+                        let memPos := mload(_2)
+                        mstore(memPos, _7)
+                        let length := mload(memPtr_1)
+                        mstore(add(memPos, _7), length)
+                        let i := _3
+                        for { } lt(i, length) { i := add(i, _7) }
+                        {
+                            mstore(add(add(memPos, i), _2), mload(add(add(memPtr_1, i), _7)))
+                        }
+                        if gt(i, length)
+                        {
+                            mstore(add(add(memPos, length), _2), _3)
+                        }
+                        return(memPos, add(sub(add(memPos, and(add(length, 31), not(31))), memPos), _2))
                     }
                 }
                 revert(0, 0)
             }
-            function abi_encode_string(headStart, value0) -> tail
-            {
-                let _1 := 32
-                mstore(headStart, _1)
-                let length := mload(value0)
-                mstore(add(headStart, _1), length)
-                let i := 0
-                for { } lt(i, length) { i := add(i, _1) }
-                {
-                    mstore(add(add(headStart, i), 64), mload(add(add(value0, i), _1)))
-                }
-                if gt(i, length)
-                {
-                    mstore(add(add(headStart, length), 64), 0)
-                }
-                tail := add(add(headStart, and(add(length, 31), not(31))), 64)
-            }
-            function panic_error_0x41()
-            {
-                mstore(0, shl(224, 0x4e487b71))
-                mstore(4, 0x41)
-                revert(0, 0x24)
-            }
-            function allocate_memory_array_string() -> memPtr
-            {
-                let memPtr_1 := mload(64)
-                let newFreePtr := add(memPtr_1, 64)
-                if or(gt(newFreePtr, 0xffffffffffffffff), lt(newFreePtr, memPtr_1)) { panic_error_0x41() }
-                mstore(64, newFreePtr)
-                memPtr := memPtr_1
-                mstore(memPtr_1, 2)
-            }
-            function allocate_memory_array_string_546() -> memPtr
-            {
-                let memPtr_1 := mload(64)
-                let newFreePtr := add(memPtr_1, 96)
-                if or(gt(newFreePtr, 0xffffffffffffffff), lt(newFreePtr, memPtr_1)) { panic_error_0x41() }
-                mstore(64, newFreePtr)
-                memPtr := memPtr_1
-                mstore(memPtr_1, 39)
-            }
         }
         /// @use-src 0:"debug_info_in_yul_snippet_escaping/input.sol"
         object "C_2" {

test/cmdlineTests/function_debug_info_via_yul/output

@@ -4,20 +4,7 @@
     "function_debug_info_via_yul/input.sol:C":
     {
       "function-debug": {},
-      "function-debug-runtime":
-      {
-        "abi_encode_uint256":
-        {
-          "parameterSlots": 2,
-          "returnSlots": 1
-        },
-        "calldata_array_index_access_uint256_dyn_calldata":
-        {
-          "entryPoint": 144,
-          "parameterSlots": 2,
-          "returnSlots": 1
-        }
-      }
+      "function-debug-runtime": {}
     }
   },
   "version": "<VERSION REMOVED>"