New simplifier via broken expressions.

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/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,7 @@ 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 == "unusedPruner")
 	{