Merge pull request #5147 from ethereum/simplifierViaBroken

[Yul] Simplifier via broken

libyul/optimiser/ExpressionSimplifier.cpp

@@ -22,6 +22,7 @@
 
 #include <libyul/optimiser/SimplificationRules.h>
 #include <libyul/optimiser/Semantics.h>
+#include <libyul/optimiser/SSAValueTracker.h>
 
 #include <libsolidity/inlineasm/AsmData.h>
 
@@ -36,13 +37,24 @@ using namespace dev::solidity;
 void ExpressionSimplifier::visit(Expression& _expression)
 {
 	ASTModifier::visit(_expression);
-	while (auto match = SimplificationRules::findFirstMatch(_expression))
+	while (auto match = SimplificationRules::findFirstMatch(_expression, m_ssaValues))
 	{
 		// Do not apply the rule if it removes non-constant parts of the expression.
 		// TODO: The check could actually be less strict than "movable".
 		// We only require "Does not cause side-effects".
+		// Note: References to variables that are only assigned once are always movable,
+		// so if the value of the variable is not movable, the expression that references
+		// the variable still is.
+
 		if (match->removesNonConstants && !MovableChecker(_expression).movable())
 			return;
 		_expression = match->action().toExpression(locationOf(_expression));
 	}
 }
+
+void ExpressionSimplifier::run(Block& _ast)
+{
+	SSAValueTracker ssaValues;
+	ssaValues(_ast);
+	ExpressionSimplifier{ssaValues.values()}(_ast);
+}

libyul/optimiser/ExpressionSimplifier.h

@@ -31,6 +31,10 @@ namespace yul
 
 /**
  * Applies simplification rules to all expressions.
+ * The component will work best if the code is in SSA form, but
+ * this is not required for correctness.
+ *
+ * Prerequisite: Disambiguator.
  */
 class ExpressionSimplifier: public ASTModifier
 {
@@ -38,7 +42,13 @@ public:
 	using ASTModifier::operator();
 	virtual void visit(Expression& _expression);
 
+	static void run(Block& _ast);
 private:
+	explicit ExpressionSimplifier(std::map<std::string, Expression const*> _ssaValues):
+		m_ssaValues(std::move(_ssaValues))
+	{}
+
+	std::map<std::string, Expression const*> m_ssaValues;
 };
 
 }

libyul/optimiser/SSAValueTracker.cpp

@@ -0,0 +1,53 @@
+/*
+	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/>.
+*/
+/**
+ * Component that collects variables that are never assigned to and their
+ * initial values.
+ */
+
+#include <libyul/optimiser/SSAValueTracker.h>
+
+#include <libsolidity/inlineasm/AsmData.h>
+
+using namespace std;
+using namespace dev;
+using namespace dev::yul;
+
+void SSAValueTracker::operator()(Assignment const& _assignment)
+{
+	for (auto const& var: _assignment.variableNames)
+		m_values.erase(var.name);
+}
+
+void SSAValueTracker::operator()(VariableDeclaration const& _varDecl)
+{
+	if (_varDecl.variables.size() == 1)
+		setValue(_varDecl.variables.front().name, _varDecl.value.get());
+	else
+		for (auto const& var: _varDecl.variables)
+			setValue(var.name, nullptr);
+}
+
+void SSAValueTracker::setValue(string const& _name, Expression const* _value)
+{
+	assertThrow(
+		m_values.count(_name) == 0,
+		OptimizerException,
+		"Source needs to be disambiguated."
+	);
+	m_values[_name] = _value;
+}

libyul/optimiser/SSAValueTracker.h

@@ -0,0 +1,58 @@
+/*
+	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/>.
+*/
+/**
+ * Component that collects variables that are never assigned to and their
+ * initial values.
+ */
+
+#pragma once
+
+#include <libyul/optimiser/ASTWalker.h>
+
+#include <string>
+#include <map>
+#include <set>
+
+namespace dev
+{
+namespace yul
+{
+
+/**
+ * Class that walks the AST and stores the initial value of each variable
+ * that is never assigned to.
+ *
+ * Prerequisite: Disambiguator
+ */
+class SSAValueTracker: public ASTWalker
+{
+public:
+	using ASTWalker::operator();
+	virtual void operator()(VariableDeclaration const& _varDecl) override;
+	virtual void operator()(Assignment const& _assignment) override;
+
+	std::map<std::string, Expression const*> const& values() const { return m_values; }
+	Expression const* value(std::string const& _name) const { return m_values.at(_name); }
+
+private:
+	void setValue(std::string const& _name, Expression const* _value);
+
+	std::map<std::string, Expression const*> m_values;
+};
+
+}
+}

libyul/optimiser/SimplificationRules.cpp

@@ -34,7 +34,10 @@ using namespace dev;
 using namespace dev::yul;
 
 
-SimplificationRule<Pattern> const* SimplificationRules::findFirstMatch(Expression const& _expr)
+SimplificationRule<Pattern> const* SimplificationRules::findFirstMatch(
+	Expression const& _expr,
+	map<string, Expression const*> const& _ssaValues
+)
 {
 	if (_expr.type() != typeid(FunctionalInstruction))
 		return nullptr;
@@ -46,7 +49,7 @@ SimplificationRule<Pattern> const* SimplificationRules::findFirstMatch(Expressio
 	for (auto const& rule: rules.m_rules[byte(instruction.instruction)])
 	{
 		rules.resetMatchGroups();
-		if (rule.pattern.matches(_expr))
+		if (rule.pattern.matches(_expr, _ssaValues))
 			return &rule;
 	}
 	return nullptr;
@@ -101,13 +104,25 @@ void Pattern::setMatchGroup(unsigned _group, map<unsigned, Expression const*>& _
 	m_matchGroups = &_matchGroups;
 }
 
-bool Pattern::matches(Expression const& _expr) const
+bool Pattern::matches(Expression const& _expr, map<string, Expression const*> const& _ssaValues) const
 {
+	Expression const* expr = &_expr;
+
+	// Resolve the variable if possible.
+	// Do not do it for "Any" because we can check identity better for variables.
+	if (m_kind != PatternKind::Any && _expr.type() == typeid(Identifier))
+	{
+		string const& varName = boost::get<Identifier>(_expr).name;
+		if (_ssaValues.count(varName))
+			expr = _ssaValues.at(varName);
+	}
+	assertThrow(expr, OptimizerException, "");
+
 	if (m_kind == PatternKind::Constant)
 	{
-		if (_expr.type() != typeid(Literal))
+		if (expr->type() != typeid(Literal))
 			return false;
-		Literal const& literal = boost::get<Literal>(_expr);
+		Literal const& literal = boost::get<Literal>(*expr);
 		if (literal.kind != assembly::LiteralKind::Number)
 			return false;
 		if (m_data && *m_data != u256(literal.value))
@@ -116,34 +131,51 @@ bool Pattern::matches(Expression const& _expr) const
 	}
 	else if (m_kind == PatternKind::Operation)
 	{
-		if (_expr.type() != typeid(FunctionalInstruction))
+		if (expr->type() != typeid(FunctionalInstruction))
 			return false;
-		FunctionalInstruction const& instr = boost::get<FunctionalInstruction>(_expr);
+		FunctionalInstruction const& instr = boost::get<FunctionalInstruction>(*expr);
 		if (m_instruction != instr.instruction)
 			return false;
 		assertThrow(m_arguments.size() == instr.arguments.size(), OptimizerException, "");
 		for (size_t i = 0; i < m_arguments.size(); ++i)
-			if (!m_arguments[i].matches(instr.arguments.at(i)))
+			if (!m_arguments[i].matches(instr.arguments.at(i), _ssaValues))
 				return false;
 	}
 	else
 	{
-		assertThrow(m_arguments.empty(), OptimizerException, "");
+		assertThrow(m_arguments.empty(), OptimizerException, "\"Any\" should not have arguments.");
 	}
-	// We support matching multiple expressions that require the same value
-	// based on identical ASTs, which have to be movable.
+
 	if (m_matchGroup)
 	{
+		// We support matching multiple expressions that require the same value
+		// based on identical ASTs, which have to be movable.
+
+		// TODO: add tests:
+		// - { let x := mload(0) let y := and(x, x) }
+		// - { let x := 4 let y := and(x, y) }
+
+		// This code uses `_expr` again for "Any", because we want the comparison to be done
+		// on the variables and not their values.
+		// The assumption is that CSE or local value numbering has been done prior to this step.
+
 		if (m_matchGroups->count(m_matchGroup))
 		{
+			assertThrow(m_kind == PatternKind::Any, OptimizerException, "Match group repetition for non-any.");
 			Expression const* firstMatch = (*m_matchGroups)[m_matchGroup];
 			assertThrow(firstMatch, OptimizerException, "Match set but to null.");
 			return
 				SyntacticalEqualityChecker::equal(*firstMatch, _expr) &&
 				MovableChecker(_expr).movable();
 		}
-		else
+		else if (m_kind == PatternKind::Any)
 			(*m_matchGroups)[m_matchGroup] = &_expr;
+		else
+		{
+			assertThrow(m_kind == PatternKind::Constant, OptimizerException, "Match group set for operation.");
+			// We do not use _expr here, because we want the actual number.
+			(*m_matchGroups)[m_matchGroup] = expr;
+		}
 	}
 	return true;
 }

libyul/optimiser/SimplificationRules.h

@@ -49,7 +49,11 @@ public:
 
 	/// @returns a pointer to the first matching pattern and sets the match
 	/// groups accordingly.
-	static SimplificationRule<Pattern> const* findFirstMatch(Expression const& _expr);
+	/// @param _ssaValues values of variables that are assigned exactly once.
+	static SimplificationRule<Pattern> const* findFirstMatch(
+		Expression const& _expr,
+		std::map<std::string, Expression const*> const& _ssaValues
+	);
 
 	/// Checks whether the rulelist is non-empty. This is usually enforced
 	/// by the constructor, but we had some issues with static initialization.
@@ -92,7 +96,7 @@ public:
 	/// same expression equivalence class.
 	void setMatchGroup(unsigned _group, std::map<unsigned, Expression const*>& _matchGroups);
 	unsigned matchGroup() const { return m_matchGroup; }
-	bool matches(Expression const& _expr) const;
+	bool matches(Expression const& _expr, std::map<std::string, Expression const*> const& _ssaValues) const;
 
 	std::vector<Pattern> arguments() const { return m_arguments; }
 

test/libyul/YulOptimizerTest.cpp

@@ -146,7 +146,19 @@ bool YulOptimizerTest::run(ostream& _stream, string const& _linePrefix, bool con
 	else if (m_optimizerStep == "expressionSimplifier")
 	{
 		disambiguate();
-		(ExpressionSimplifier{})(*m_ast);
+		ExpressionSimplifier::run(*m_ast);
+	}
+	else if (m_optimizerStep == "fullSimplify")
+	{
+		disambiguate();
+		NameDispenser nameDispenser;
+		nameDispenser.m_usedNames = NameCollector(*m_ast).names();
+		ExpressionSplitter{nameDispenser}(*m_ast);
+		CommonSubexpressionEliminator{}(*m_ast);
+		ExpressionSimplifier::run(*m_ast);
+		UnusedPruner::runUntilStabilised(*m_ast);
+		ExpressionJoiner::run(*m_ast);
+		ExpressionJoiner::run(*m_ast);
 	}
 	else if (m_optimizerStep == "unusedPruner")
 	{

test/libyul/yulOptimizerTests/fullSimplify/constant_propagation.yul

@@ -0,0 +1,10 @@
+{
+    let a := add(7, sub(mload(0), 7))
+    mstore(a, 0)
+}
+// ----
+// fullSimplify
+// {
+//     let _2 := 0
+//     mstore(mload(_2), _2)
+// }

test/libyul/yulOptimizerTests/fullSimplify/constants.yul

@@ -0,0 +1,9 @@
+{
+    let a := add(1, mul(3, 4))
+    mstore(0, a)
+}
+// ----
+// fullSimplify
+// {
+//     mstore(0, 13)
+// }

test/libyul/yulOptimizerTests/fullSimplify/identity_rules_complex.yul

@@ -0,0 +1,9 @@
+{
+    let a := sub(calldataload(0), calldataload(0))
+    mstore(a, 0)
+}
+// ----
+// fullSimplify
+// {
+//     mstore(0, 0)
+// }

test/libyul/yulOptimizerTests/fullSimplify/identity_rules_negative.yul

@@ -0,0 +1,10 @@
+{
+    let a := sub(calldataload(1), calldataload(0))
+    mstore(0, a)
+}
+// ----
+// fullSimplify
+// {
+//     let _1 := 0
+//     mstore(_1, sub(calldataload(1), calldataload(_1)))
+// }

test/libyul/yulOptimizerTests/fullSimplify/identity_rules_simple.yul

@@ -0,0 +1,11 @@
+{
+    let a := mload(0)
+    mstore(0, sub(a, a))
+}
+// ----
+// fullSimplify
+// {
+//     let _1 := 0
+//     pop(mload(_1))
+//     mstore(_1, 0)
+// }

test/libyul/yulOptimizerTests/fullSimplify/including_function_calls.yul

@@ -0,0 +1,13 @@
+{
+    function f() -> a {}
+    let b := add(7, sub(f(), 7))
+    mstore(b, 0)
+}
+// ----
+// fullSimplify
+// {
+//     function f() -> a
+//     {
+//     }
+//     mstore(f(), 0)
+// }

test/libyul/yulOptimizerTests/fullSimplify/inside_for.yul

@@ -0,0 +1,17 @@
+{
+    let x := calldataload(3)
+    for { let a := 10 } iszero(eq(a, sub(x, calldataload(3)))) { a := add(a, 1) } {}
+}
+// ----
+// fullSimplify
+// {
+//     for {
+//         let a := 10
+//     }
+//     iszero(iszero(a))
+//     {
+//         a := add(a, 1)
+//     }
+//     {
+//     }
+// }

test/libyul/yulOptimizerTests/fullSimplify/invariant.yul

@@ -0,0 +1,17 @@
+{
+    let a := calldataload(sub(7, 7))
+    let b := sub(a, 0)
+    // Below, `b` is not eliminated, because
+    // we run CSE and then Simplify.
+    // Elimination of `b` would require another
+    // run of CSE afterwards.
+    mstore(b, eq(calldataload(0), a))
+}
+// ----
+// fullSimplify
+// {
+//     let a := calldataload(0)
+//     let _4 := 0
+//     let b := a
+//     mstore(b, eq(calldataload(_4), a))
+// }

test/libyul/yulOptimizerTests/fullSimplify/mod_and_1.yul

@@ -0,0 +1,9 @@
+{
+    mstore(0, mod(calldataload(0), exp(2, 8)))
+}
+// ----
+// fullSimplify
+// {
+//     let _4 := 0
+//     mstore(_4, and(calldataload(_4), 255))
+// }

test/libyul/yulOptimizerTests/fullSimplify/mod_and_2.yul

@@ -0,0 +1,9 @@
+{
+    mstore(0, mod(calldataload(0), exp(2, 255)))
+}
+// ----
+// fullSimplify
+// {
+//     let _4 := 0
+//     mstore(_4, and(calldataload(_4), 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff))
+// }

test/libyul/yulOptimizerTests/fullSimplify/not_applied_function_call_different_arguments.yul

@@ -0,0 +1,14 @@
+{
+    function f(a) -> b { }
+    mstore(0, sub(f(0), f(1)))
+}
+// ----
+// fullSimplify
+// {
+//     function f(a) -> b
+//     {
+//     }
+//     let _2 := f(1)
+//     let _3 := 0
+//     mstore(_3, sub(f(_3), _2))
+// }

test/libyul/yulOptimizerTests/fullSimplify/not_applied_function_call_different_names.yul

@@ -0,0 +1,17 @@
+{
+	function f1() -> a { }
+	function f2() -> b { }
+	let c := sub(f1(), f2())
+	mstore(0, c)
+}
+// ----
+// fullSimplify
+// {
+//     function f1() -> a
+//     {
+//     }
+//     function f2() -> b
+//     {
+//     }
+//     mstore(0, sub(f1(), f2()))
+// }

test/libyul/yulOptimizerTests/fullSimplify/not_applied_function_call_equality_not_movable.yul

@@ -0,0 +1,14 @@
+// Even if the functions pass the equality check, they are not movable.
+{
+	function f() -> a { }
+	let b := sub(f(), f())
+	mstore(0, b)
+}
+// ----
+// fullSimplify
+// {
+//     function f() -> a
+//     {
+//     }
+//     mstore(0, sub(f(), f()))
+// }

test/libyul/yulOptimizerTests/fullSimplify/not_applied_removes_non_constant_and_not_movable.yul

@@ -0,0 +1,12 @@
+// div is eliminated, but keccak256 has side-effects.
+{
+	let a := div(keccak256(0, 0), 0)
+	mstore(0, a)
+}
+// ----
+// fullSimplify
+// {
+//     let _1 := 0
+//     pop(keccak256(_1, _1))
+//     mstore(_1, 0)
+// }

test/libyul/yulOptimizerTests/fullSimplify/reversed.yul

@@ -0,0 +1,10 @@
+{
+    let a := add(0, mload(0))
+    mstore(0, a)
+}
+// ----
+// fullSimplify
+// {
+//     let _1 := 0
+//     mstore(_1, mload(_1))
+// }

test/libyul/yulOptimizerTests/fullSimplify/smoke.yul

@@ -0,0 +1,5 @@
+{ }
+// ----
+// fullSimplify
+// {
+// }