Merge pull request #6206 from ethereum/switchSimplifierFix

Undo switch simplifier force-push trouble

Changelog.md

@@ -5,6 +5,9 @@ Language Features:
 
 Compiler Features:
  * SMTChecker: Support one-dimensional arrays.
+ * Yul Optimizer: Add rule to remove empty default switch cases
+ * Yul Optimizer: Add rule to remove empty cases if no default exists
+ * Yul Optimizer: Add rule to replace a switch with no cases with pop(expression)
 
 
 Bugfixes:

libyul/optimiser/StructuralSimplifier.cpp

@@ -22,12 +22,14 @@
 #include <libdevcore/Visitor.h>
 
 #include <boost/range/algorithm_ext/erase.hpp>
-#include <boost/range/algorithm/find_if.hpp>
+#include <boost/algorithm/cxx11/any_of.hpp>
 
 using namespace std;
 using namespace dev;
 using namespace yul;
 
+using OptionalStatements = boost::optional<vector<Statement>>;
+
 namespace {
 
 ExpressionStatement makePopExpressionStatement(langutil::SourceLocation const& _location, Expression&& _expression)
@@ -39,6 +41,84 @@ ExpressionStatement makePopExpressionStatement(langutil::SourceLocation const& _
 	}};
 }
 
+void removeEmptyDefaultFromSwitch(Switch& _switchStmt)
+{
+	boost::remove_erase_if(
+		_switchStmt.cases,
+		[](Case const& _case) { return !_case.value && _case.body.statements.empty(); }
+	);
+}
+
+void removeEmptyCasesFromSwitch(Switch& _switchStmt)
+{
+	bool hasDefault = boost::algorithm::any_of(
+		_switchStmt.cases,
+		[](Case const& _case) { return !_case.value; }
+	);
+
+	if (hasDefault)
+		return;
+
+	boost::remove_erase_if(
+		_switchStmt.cases,
+		[](Case const& _case) { return _case.body.statements.empty(); }
+	);
+}
+
+OptionalStatements replaceConstArgSwitch(Switch& _switchStmt, u256 const& _constExprVal)
+{
+	Block* matchingCaseBlock = nullptr;
+	Case* defaultCase = nullptr;
+
+	for (auto& _case: _switchStmt.cases)
+	{
+		if (_case.value && valueOfLiteral(*_case.value) == _constExprVal)
+		{
+			matchingCaseBlock = &_case.body;
+			break;
+		}
+		else if (!_case.value)
+			defaultCase = &_case;
+	}
+
+	if (!matchingCaseBlock && defaultCase)
+		matchingCaseBlock = &defaultCase->body;
+
+	OptionalStatements s = vector<Statement>{};
+
+	if (matchingCaseBlock)
+		s->emplace_back(std::move(*matchingCaseBlock));
+
+	return s;
+}
+
+OptionalStatements reduceSingleCaseSwitch(Switch& _switchStmt)
+{
+	yulAssert(_switchStmt.cases.size() == 1, "Expected only one case!");
+
+	auto& switchCase = _switchStmt.cases.front();
+	auto loc = locationOf(*_switchStmt.expression);
+	if (switchCase.value)
+	{
+		OptionalStatements s = vector<Statement>{};
+		s->emplace_back(If{
+				std::move(_switchStmt.location),
+				make_unique<Expression>(FunctionalInstruction{
+						std::move(loc),
+						solidity::Instruction::EQ,
+						{std::move(*switchCase.value), std::move(*_switchStmt.expression)}
+						}), std::move(switchCase.body)});
+		return s;
+	}
+	else
+	{
+		OptionalStatements s = vector<Statement>{};
+		s->emplace_back(makePopExpressionStatement(loc, std::move(*_switchStmt.expression)));
+		s->emplace_back(std::move(switchCase.body));
+		return s;
+	}
+}
+
 }
 
 void StructuralSimplifier::operator()(Block& _block)
@@ -48,6 +128,19 @@ void StructuralSimplifier::operator()(Block& _block)
 	popScope();
 }
 
+OptionalStatements StructuralSimplifier::reduceNoCaseSwitch(Switch& _switchStmt) const
+{
+	yulAssert(_switchStmt.cases.empty(), "Expected no case!");
+
+	auto loc = locationOf(*_switchStmt.expression);
+
+	OptionalStatements s = vector<Statement>{};
+
+	s->emplace_back(makePopExpressionStatement(loc, std::move(*_switchStmt.expression)));
+
+	return s;
+}
+
 boost::optional<dev::u256> StructuralSimplifier::hasLiteralValue(Expression const& _expression) const
 {
 	Expression const* expr = &_expression;
@@ -70,7 +163,6 @@ boost::optional<dev::u256> StructuralSimplifier::hasLiteralValue(Expression cons
 
 void StructuralSimplifier::simplify(std::vector<yul::Statement>& _statements)
 {
-	using OptionalStatements = boost::optional<vector<Statement>>;
 	GenericFallbackReturnsVisitor<OptionalStatements, If, Switch, ForLoop> const visitor(
 		[&](If& _ifStmt) -> OptionalStatements {
 			if (_ifStmt.body.statements.empty())
@@ -86,75 +178,15 @@ void StructuralSimplifier::simplify(std::vector<yul::Statement>& _statements)
 			return {};
 		},
 		[&](Switch& _switchStmt) -> OptionalStatements {
-			auto& cases = _switchStmt.cases;
+			removeEmptyDefaultFromSwitch(_switchStmt);
+			removeEmptyCasesFromSwitch(_switchStmt);
 
-			if (cases.size() == 1)
+			if (_switchStmt.cases.empty())
-			{
+				return reduceNoCaseSwitch(_switchStmt);
-				auto& switchCase = cases.front();
+			else if (_switchStmt.cases.size() == 1)
-				auto loc = locationOf(*_switchStmt.expression);
+				return reduceSingleCaseSwitch(_switchStmt);
-				if (switchCase.value)
-				{
-					OptionalStatements s = vector<Statement>{};
-					s->emplace_back(If{
-						std::move(_switchStmt.location),
-						make_unique<Expression>(FunctionalInstruction{
-								std::move(loc),
-								solidity::Instruction::EQ,
-								{std::move(*switchCase.value), std::move(*_switchStmt.expression)}
-						}), std::move(switchCase.body)});
-					return s;
-				}
-				else
-				{
-					OptionalStatements s = vector<Statement>{};
-					s->emplace_back(makePopExpressionStatement(loc, std::move(*_switchStmt.expression)));
-					s->emplace_back(std::move(switchCase.body));
-					return s;
-				}
-			}
-			// Replace the whole switch with the resulting case body if arg. is
-			// a constant
 			else if (boost::optional<u256> const constExprVal = hasLiteralValue(*_switchStmt.expression))
-			{
+				return replaceConstArgSwitch(_switchStmt, constExprVal.get());
-				Block* matchingCaseBlock = nullptr;
-				Case* defaultCase = nullptr;
-
-				for (auto& _case: cases)
-				{
-					if (_case.value && valueOfLiteral(*_case.value) == constExprVal)
-					{
-						matchingCaseBlock = &_case.body;
-						break;
-					}
-					else if (!_case.value)
-						defaultCase = &_case;
-				}
-
-				if (!matchingCaseBlock && defaultCase)
-					matchingCaseBlock = &defaultCase->body;
-
-				OptionalStatements s = vector<Statement>{};
-
-				if (matchingCaseBlock)
-					s->emplace_back(std::move(*matchingCaseBlock));
-
-				return s;
-			}
-
-			// Remove cases with empty body if no default case exists
-			auto const defaultCase = boost::find_if(
-				cases,
-				[](Case const& _case) { return !_case.value; });
-
-			if (
-				(defaultCase != cases.end() &&
-				defaultCase->body.statements.empty()) ||
-				defaultCase == cases.end()
-			)
-				boost::remove_erase_if(
-					cases,
-					[](Case const& _case) { return _case.body.statements.empty(); }
-				);
 
 			return {};
 		},

libyul/optimiser/StructuralSimplifier.h

@@ -28,6 +28,9 @@ namespace yul
  * - replace if with empty body with pop(condition)
  * - replace if with true condition with its body
  * - remove if with false condition
+ * - remove empty default switch case
+ * - remove empty switch case if no default case exists
+ * - replace switch with no cases with pop(expression)
  * - turn switch with single case into if
  * - replace switch with only default case with pop(expression) and body
  * - replace switch with const expr with matching case body
@@ -49,6 +52,7 @@ private:
 	bool expressionAlwaysTrue(Expression const& _expression);
 	bool expressionAlwaysFalse(Expression const& _expression);
 	boost::optional<dev::u256> hasLiteralValue(Expression const& _expression) const;
+	boost::optional<std::vector<Statement>> reduceNoCaseSwitch(Switch& _switchStmt) const;
 };
 
 }

test/libyul/yulOptimizerTests/structuralSimplifier/switch_remove_empty_all.yul

@@ -0,0 +1,19 @@
+{
+	let y := 200
+	switch add(y, 4)
+	case 0 { }
+	case 1 { }
+	default { }
+
+	switch 4
+	case 0 { }
+	case 1 { }
+	default { }
+}
+// ----
+// structuralSimplifier
+// {
+//     let y := 200
+//     pop(add(y, 4))
+//     pop(4)
+// }

test/libyul/yulOptimizerTests/structuralSimplifier/switch_remove_empty_case.yul

@@ -8,8 +8,8 @@
 // structuralSimplifier
 // {
 //     let y := 200
-//     switch y
+//     if eq(1, y)
-//     case 1 {
+//     {
 //         y := 9
 //     }
 // }

test/libyul/yulOptimizerTests/structuralSimplifier/switch_remove_empty_cases.yul

@@ -9,8 +9,8 @@
 // structuralSimplifier
 // {
 //     let y := 200
-//     switch y
+//     if eq(1, y)
-//     case 1 {
+//     {
 //         y := 9
 //     }
 // }

test/libyul/yulOptimizerTests/structuralSimplifier/switch_remove_empty_default_case.yul

@@ -8,8 +8,8 @@
 // structuralSimplifier
 // {
 //     let y := 200
-//     switch y
+//     if eq(1, y)
-//     case 1 {
+//     {
 //         y := 9
 //     }
 // }