Merge pull request #7122 from sifmelcara/expression-inliner-avoid-dup-expr

[Yul] ExpressionInliner: avoid duplicating expressions

Changelog.md

@@ -14,6 +14,7 @@ Compiler Features:
  * Standard JSON Interface: Compile only selected sources and contracts.
  * Standard JSON Interface: Provide secondary error locations (e.g. the source position of other conflicting declarations).
  * SMTChecker: Do not erase knowledge about storage pointers if another storage pointer is assigned.
+ * Yul Optimizer: Do not inline function if it would result in expressions being duplicated that are not cheap.
 
 
 

libyul/optimiser/ExpressionInliner.cpp

@@ -21,11 +21,12 @@
 #include <libyul/optimiser/ExpressionInliner.h>
 
 #include <libyul/optimiser/InlinableExpressionFunctionFinder.h>
+#include <libyul/optimiser/Metrics.h>
+#include <libyul/optimiser/NameCollector.h>
 #include <libyul/optimiser/Substitution.h>
 #include <libyul/optimiser/Semantics.h>
-#include <libyul/AsmData.h>
 
-#include <boost/algorithm/cxx11/all_of.hpp>
+#include <libyul/AsmData.h>
 
 using namespace std;
 using namespace dev;
@@ -40,7 +41,6 @@ void ExpressionInliner::run()
 	(*this)(m_block);
 }
 
-
 void ExpressionInliner::operator()(FunctionDefinition& _fun)
 {
 	ASTModifier::operator()(_fun);
@@ -52,21 +52,28 @@ void ExpressionInliner::visit(Expression& _expression)
 	if (_expression.type() == typeid(FunctionCall))
 	{
 		FunctionCall& funCall = boost::get<FunctionCall>(_expression);
+		if (!m_inlinableFunctions.count(funCall.functionName.name))
+			return;
+		FunctionDefinition const& fun = *m_inlinableFunctions.at(funCall.functionName.name);
 
-		bool movable = boost::algorithm::all_of(
-			funCall.arguments,
-			[=](Expression const& _arg) { return SideEffectsCollector(m_dialect, _arg).movable(); }
-		);
-		if (m_inlinableFunctions.count(funCall.functionName.name) && movable)
+		map<YulString, Expression const*> substitutions;
+		for (size_t i = 0; i < funCall.arguments.size(); i++)
 		{
-			FunctionDefinition const& fun = *m_inlinableFunctions.at(funCall.functionName.name);
-			map<YulString, Expression const*> substitutions;
-			for (size_t i = 0; i < fun.parameters.size(); ++i)
-				substitutions[fun.parameters[i].name] = &funCall.arguments[i];
-			_expression = Substitution(substitutions).translate(*boost::get<Assignment>(fun.body.statements.front()).value);
+			Expression const& arg = funCall.arguments[i];
+			YulString paraName = fun.parameters[i].name;
 
-			// TODO Add metric! This metric should perform well on a pair of functions who
-			// call each other recursively.
+			if (!SideEffectsCollector(m_dialect, arg).movable())
+				return;
+
+			size_t refs = ReferencesCounter::countReferences(fun.body)[paraName];
+			size_t cost = CodeCost::codeCost(m_dialect, arg);
+
+			if (refs > 1 && cost > 1)
+				return;
+
+			substitutions[paraName] = &arg;
 		}
+
+		_expression = Substitution(substitutions).translate(*boost::get<Assignment>(fun.body.statements.front()).value);
 	}
 }

libyul/optimiser/ExpressionInliner.h

@@ -38,7 +38,10 @@ struct Dialect;
  *  - have a body like r := <functional expression>
  *  - neither reference themselves nor r in the right hand side
  *
- * Furthermore, the arguments of the function call cannot have any side-effects.
+ * Furthermore, for all parameters, all of the following need to be true
+ *  - the argument is movable
+ *  - the parameter is either referenced less than twice in the function body, or the argument is rather cheap
+ *    ("cost" of at most 1 like a constant up to 0xff)
  *
  * This component can only be used on sources with unique names.
  */
@@ -66,5 +69,4 @@ private:
 	Dialect const& m_dialect;
 };
 
-
 }

test/libyul/yulOptimizerTests/expressionInliner/argument_duplication_heuristic.yul

@@ -0,0 +1,41 @@
+{
+    // references parameter 1 times in function body
+    function ref1(a) -> x { x := add(a, 1) }
+    // references parameter 3 times in function body
+    function ref3(a) -> x { x := add(a, mul(a, a)) }
+    let y1 := ref1(calldatasize())
+    let y2 := ref3(calldatasize())
+    let y3 := ref1(0xff)
+    let y4 := ref3(0xff)
+    let y5 := ref1(0x123)
+    let y6 := ref3(0x123)
+    let y7 := ref1(mload(42))
+    let y8 := ref3(mload(42))
+    let y9 := ref1(ref3(7))
+    let y10:= ref3(ref1(7))
+    let y11:= ref1(y1)
+    let y12:= ref3(y1)
+}
+// ====
+// step: expressionInliner
+// ----
+// {
+//     function ref1(a) -> x
+//     { x := add(a, 1) }
+//     function ref3(a_1) -> x_2
+//     {
+//         x_2 := add(a_1, mul(a_1, a_1))
+//     }
+//     let y1 := add(calldatasize(), 1)
+//     let y2 := add(calldatasize(), mul(calldatasize(), calldatasize()))
+//     let y3 := add(0xff, 1)
+//     let y4 := add(0xff, mul(0xff, 0xff))
+//     let y5 := add(0x123, 1)
+//     let y6 := ref3(0x123)
+//     let y7 := ref1(mload(42))
+//     let y8 := ref3(mload(42))
+//     let y9 := add(add(7, mul(7, 7)), 1)
+//     let y10 := ref3(add(7, 1))
+//     let y11 := add(y1, 1)
+//     let y12 := add(y1, mul(y1, y1))
+// }