Merge pull request #5205 from ethereum/cseAlsoSubstituteVariables

[Yul] Also substitute variables in Common Subexpression Eliminator

3 files changed, 67 insertions, 4 deletions

diff --git a/libjulia/optimiser/CommonSubexpressionEliminator.cpp b/libjulia/optimiser/CommonSubexpressionEliminator.cpp
index 3122280b..458b3437 100644
--- a/libjulia/optimiser/CommonSubexpressionEliminator.cpp
+++ b/libjulia/optimiser/CommonSubexpressionEliminator.cpp
@@ -33,9 +33,31 @@ using namespace dev::julia;
 
 void CommonSubexpressionEliminator::visit(Expression& _e)
 {
-	// Single exception for substitution: We do not substitute one variable for another.
-	if (_e.type() != typeid(Identifier))
-		// TODO this search rather inefficient.
+	// We visit the inner expression first to first simplify inner expressions,
+	// which hopefully allows more matches.
+	// Note that the DataFlowAnalyzer itself only has code for visiting Statements,
+	// so this basically invokes the AST walker directly and thus post-visiting
+	// is also fine with regards to data flow analysis.
+	DataFlowAnalyzer::visit(_e);
+
+	if (_e.type() == typeid(Identifier))
+	{
+		Identifier& identifier = boost::get<Identifier>(_e);
+		string const& name = identifier.name;
+		if (m_value.count(name))
+		{
+			assertThrow(m_value.at(name), OptimizerException, "");
+			if (m_value.at(name)->type() == typeid(Identifier))
+			{
+				string const& value = boost::get<Identifier>(*m_value.at(name)).name;
+				if (inScope(value))
+					_e = Identifier{locationOf(_e), value};
+			}
+		}
+	}
+	else
+	{
+		// TODO this search is rather inefficient.
 		for (auto const& var: m_value)
 		{
 			assertThrow(var.second, OptimizerException, "");
@@ -45,5 +67,5 @@ void CommonSubexpressionEliminator::visit(Expression& _e)
 				break;
 			}
 		}
-	DataFlowAnalyzer::visit(_e);
+	}
 }
diff --git a/libjulia/optimiser/README.md b/libjulia/optimiser/README.md
index 4a50d5ca..faef818b 100644
--- a/libjulia/optimiser/README.md
+++ b/libjulia/optimiser/README.md
@@ -97,6 +97,20 @@ of any function call or opcode execution, the transformation is not performed.
 Note that the component will not move the assigned value of a variable assignment
 or a variable that is referenced more than once.
 
+## Common Subexpression Eliminator
+
+This step replaces a subexpression by the value of a pre-existing variable
+that currently has the same value (only if the value is movable), based
+on a syntactic comparison.
+
+This can be used to compute a local value numbering, especially if the
+expression splitter is used before.
+
+The expression simplifier will be able to perform better replacements
+if the common subexpression eliminator was run right before it.
+
+Prerequisites: Disambiguator
+
 ## Full Function Inliner
 
 ## Rematerialisation
diff --git a/test/libjulia/yulOptimizerTests/commonSubexpressionEliminator/variable_for_variable.yul b/test/libjulia/yulOptimizerTests/commonSubexpressionEliminator/variable_for_variable.yul
new file mode 100644
index 00000000..ab94afc2
--- /dev/null
+++ b/test/libjulia/yulOptimizerTests/commonSubexpressionEliminator/variable_for_variable.yul
@@ -0,0 +1,27 @@
+{
+
+    let a := mload(0)
+    let b := add(a, 7)
+    let c := a
+    let d := c
+    let x := add(a, b)
+    // CSE has to recognize equality with x here.
+    let y := add(d, add(c, 7))
+    // some reassignments
+    b := mload(a)
+    a := b
+    mstore(2, a)
+}
+// ----
+// commonSubexpressionEliminator
+// {
+//     let a := mload(0)
+//     let b := add(a, 7)
+//     let c := a
+//     let d := a
+//     let x := add(a, b)
+//     let y := x
+//     b := mload(a)
+//     a := b
+//     mstore(2, b)
+// }