Merge pull request #6840 from ethereum/discoverMSize

Only remove msize-modifying instructions if msize is not used.

libyul/Dialect.h

@@ -51,6 +51,11 @@ struct BuiltinFunction
 	bool movable = false;
 	/// If true, a call to this function can be omitted without changing semantics.
 	bool sideEffectFree = false;
+	/// If true, a call to this function can be omitted without changing semantics if the
+	/// program does not contain the msize instruction.
+	bool sideEffectFreeIfNoMSize = false;
+	/// If true, this is the msize instruction.
+	bool isMSize = false;
 	/// If true, can only accept literals as arguments and they cannot be moved to variables.
 	bool literalArguments = false;
 };

libyul/backends/evm/EVMDialect.cpp

@@ -52,6 +52,8 @@ pair<YulString, BuiltinFunctionForEVM> createEVMFunction(
 	f.returns.resize(info.ret);
 	f.movable = eth::SemanticInformation::movable(_instruction);
 	f.sideEffectFree = eth::SemanticInformation::sideEffectFree(_instruction);
+	f.sideEffectFreeIfNoMSize = eth::SemanticInformation::sideEffectFreeIfNoMSize(_instruction);
+	f.isMSize = _instruction == dev::eth::Instruction::MSIZE;
 	f.literalArguments = false;
 	f.instruction = _instruction;
 	f.generateCode = [_instruction](
@@ -73,6 +75,7 @@ pair<YulString, BuiltinFunctionForEVM> createFunction(
 	size_t _returns,
 	bool _movable,
 	bool _sideEffectFree,
+	bool _sideEffectFreeIfNoMSize,
 	bool _literalArguments,
 	std::function<void(FunctionCall const&, AbstractAssembly&, BuiltinContext&, std::function<void()>)> _generateCode
 )
@@ -85,6 +88,8 @@ pair<YulString, BuiltinFunctionForEVM> createFunction(
 	f.movable = _movable;
 	f.literalArguments = _literalArguments;
 	f.sideEffectFree = _sideEffectFree;
+	f.sideEffectFreeIfNoMSize = _sideEffectFreeIfNoMSize;
+	f.isMSize = false;
 	f.instruction = {};
 	f.generateCode = std::move(_generateCode);
 	return {name, f};
@@ -105,7 +110,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 
 	if (_objectAccess)
 	{
-		builtins.emplace(createFunction("datasize", 1, 1, true, true, true, [](
+		builtins.emplace(createFunction("datasize", 1, 1, true, true, true, true, [](
 			FunctionCall const& _call,
 			AbstractAssembly& _assembly,
 			BuiltinContext& _context,
@@ -126,7 +131,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 				_assembly.appendDataSize(_context.subIDs.at(dataName));
 			}
 		}));
-		builtins.emplace(createFunction("dataoffset", 1, 1, true, true, true, [](
+		builtins.emplace(createFunction("dataoffset", 1, 1, true, true, true, true, [](
 			FunctionCall const& _call,
 			AbstractAssembly& _assembly,
 			BuiltinContext& _context,
@@ -147,7 +152,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 				_assembly.appendDataOffset(_context.subIDs.at(dataName));
 			}
 		}));
-		builtins.emplace(createFunction("datacopy", 3, 0, false, false, false, [](
+		builtins.emplace(createFunction("datacopy", 3, 0, false, false, false, false, [](
 			FunctionCall const&,
 			AbstractAssembly& _assembly,
 			BuiltinContext&,

libyul/backends/wasm/WasmDialect.cpp

@@ -72,5 +72,7 @@ void WasmDialect::addFunction(string _name, size_t _params, size_t _returns)
 	f.returns.resize(_returns);
 	f.movable = false;
 	f.sideEffectFree = false;
+	f.sideEffectFreeIfNoMSize = false;
+	f.isMSize = false;
 	f.literalArguments = false;
 }

libyul/optimiser/ExpressionInliner.cpp

@@ -55,7 +55,7 @@ void ExpressionInliner::visit(Expression& _expression)
 
 		bool movable = boost::algorithm::all_of(
 			funCall.arguments,
-			[=](Expression const& _arg) { return MovableChecker(m_dialect, _arg).movable(); }
+			[=](Expression const& _arg) { return SideEffectsCollector(m_dialect, _arg).movable(); }
 		);
 		if (m_inlinableFunctions.count(funCall.functionName.name) && movable)
 		{

libyul/optimiser/ExpressionSimplifier.cpp

@@ -43,7 +43,7 @@ void ExpressionSimplifier::visit(Expression& _expression)
 		// so if the value of the variable is not movable, the expression that references
 		// the variable still is.
 
-		if (match->removesNonConstants && !MovableChecker(m_dialect, _expression).movable())
+		if (match->removesNonConstants && !SideEffectsCollector(m_dialect, _expression).movable())
 			return;
 		_expression = match->action().toExpression(locationOf(_expression));
 	}

libyul/optimiser/RedundantAssignEliminator.cpp

@@ -284,7 +284,7 @@ void RedundantAssignEliminator::finalize(
 	{
 		State const state = assignment.second == State::Undecided ? _finalState : assignment.second;
 
-		if (state == State::Unused && MovableChecker{*m_dialect, *assignment.first->value}.movable())
+		if (state == State::Unused && SideEffectsCollector{*m_dialect, *assignment.first->value}.movable())
 			// TODO the only point where we actually need this
 			// to be a set is for the for loop
 			m_pendingRemovals.insert(assignment.first);

libyul/optimiser/Semantics.cpp

@@ -33,9 +33,59 @@ using namespace std;
 using namespace dev;
 using namespace yul;
 
-MovableChecker::MovableChecker(Dialect const& _dialect):
-	m_dialect(_dialect)
+SideEffectsCollector::SideEffectsCollector(Dialect const& _dialect, Expression const& _expression):
+	SideEffectsCollector(_dialect)
 {
+	visit(_expression);
+}
+
+SideEffectsCollector::SideEffectsCollector(Dialect const& _dialect, Statement const& _statement):
+	SideEffectsCollector(_dialect)
+{
+	visit(_statement);
+}
+
+SideEffectsCollector::SideEffectsCollector(Dialect const& _dialect, Block const& _ast):
+	SideEffectsCollector(_dialect)
+{
+	operator()(_ast);
+}
+
+void SideEffectsCollector::operator()(FunctionalInstruction const& _instr)
+{
+	ASTWalker::operator()(_instr);
+
+	if (!eth::SemanticInformation::movable(_instr.instruction))
+		m_movable = false;
+	if (!eth::SemanticInformation::sideEffectFree(_instr.instruction))
+		m_sideEffectFree = false;
+	if (!eth::SemanticInformation::sideEffectFreeIfNoMSize(_instr.instruction))
+		m_sideEffectFreeIfNoMSize = false;
+	if (_instr.instruction == eth::Instruction::MSIZE)
+		m_containsMSize = true;
+}
+
+void SideEffectsCollector::operator()(FunctionCall const& _functionCall)
+{
+	ASTWalker::operator()(_functionCall);
+
+	if (BuiltinFunction const* f = m_dialect.builtin(_functionCall.functionName.name))
+	{
+		if (!f->movable)
+			m_movable = false;
+		if (!f->sideEffectFree)
+			m_sideEffectFree = false;
+		if (!f->sideEffectFreeIfNoMSize)
+			m_sideEffectFreeIfNoMSize = false;
+		if (f->isMSize)
+			m_containsMSize = true;
+	}
+	else
+	{
+		m_movable = false;
+		m_sideEffectFree = false;
+		m_sideEffectFreeIfNoMSize = false;
+	}
 }
 
 MovableChecker::MovableChecker(Dialect const& _dialect, Expression const& _expression):
@@ -46,38 +96,10 @@ MovableChecker::MovableChecker(Dialect const& _dialect, Expression const& _expre
 
 void MovableChecker::operator()(Identifier const& _identifier)
 {
-	ASTWalker::operator()(_identifier);
+	SideEffectsCollector::operator()(_identifier);
 	m_variableReferences.emplace(_identifier.name);
 }
 
-void MovableChecker::operator()(FunctionalInstruction const& _instr)
-{
-	ASTWalker::operator()(_instr);
-
-	if (!eth::SemanticInformation::movable(_instr.instruction))
-		m_movable = false;
-	if (!eth::SemanticInformation::sideEffectFree(_instr.instruction))
-		m_sideEffectFree = false;
-}
-
-void MovableChecker::operator()(FunctionCall const& _functionCall)
-{
-	ASTWalker::operator()(_functionCall);
-
-	if (BuiltinFunction const* f = m_dialect.builtin(_functionCall.functionName.name))
-	{
-		if (!f->movable)
-			m_movable = false;
-		if (!f->sideEffectFree)
-			m_sideEffectFree = false;
-	}
-	else
-	{
-		m_movable = false;
-		m_sideEffectFree = false;
-	}
-}
-
 void MovableChecker::visit(Statement const&)
 {
 	assertThrow(false, OptimizerException, "Movability for statement requested.");

libyul/optimiser/Semantics.h

@@ -29,38 +29,73 @@ namespace yul
 struct Dialect;
 
 /**
- * Specific AST walker that determines whether an expression is movable.
+ * Specific AST walker that determines side-effect free-ness and movability of code.
+ * Enters into function definitions.
  */
-class MovableChecker: public ASTWalker
+class SideEffectsCollector: public ASTWalker
 {
 public:
-	explicit MovableChecker(Dialect const& _dialect);
-	MovableChecker(Dialect const& _dialect, Expression const& _expression);
+	explicit SideEffectsCollector(Dialect const& _dialect): m_dialect(_dialect) {}
+	SideEffectsCollector(Dialect const& _dialect, Expression const& _expression);
+	SideEffectsCollector(Dialect const& _dialect, Statement const& _statement);
+	SideEffectsCollector(Dialect const& _dialect, Block const& _ast);
 
-	void operator()(Identifier const& _identifier) override;
+	using ASTWalker::operator();
 	void operator()(FunctionalInstruction const& _functionalInstruction) override;
 	void operator()(FunctionCall const& _functionCall) override;
 
-	/// Disallow visiting anything apart from Expressions (this throws).
-	void visit(Statement const&) override;
-	using ASTWalker::visit;
-
 	bool movable() const { return m_movable; }
-	bool sideEffectFree() const { return m_sideEffectFree; }
-
-	std::set<YulString> const& referencedVariables() const { return m_variableReferences; }
+	bool sideEffectFree(bool _allowMSizeModification = false) const
+	{
+		if (_allowMSizeModification)
+			return sideEffectFreeIfNoMSize();
+		else
+			return m_sideEffectFree;
+	}
+	bool sideEffectFreeIfNoMSize() const { return m_sideEffectFreeIfNoMSize; }
+	bool containsMSize() const { return m_containsMSize; }
 
 private:
 	Dialect const& m_dialect;
-	/// Which variables the current expression references.
-	std::set<YulString> m_variableReferences;
 	/// Is the current expression movable or not.
 	bool m_movable = true;
 	/// Is the current expression side-effect free, i.e. can be removed
 	/// without changing the semantics.
 	bool m_sideEffectFree = true;
+	/// Is the current expression side-effect free up to msize, i.e. can be removed
+	/// without changing the semantics except for the value returned by the msize instruction.
+	bool m_sideEffectFreeIfNoMSize = true;
+	/// Does the current code contain the MSize operation?
+	/// Note that this is a purely syntactic property meaning that even if this is false,
+	/// the code can still contain calls to functions that contain the msize instruction.
+	bool m_containsMSize = false;
 };
 
+/**
+ * Specific AST walker that determines whether an expression is movable
+ * and collects the referenced variables.
+ * Can only be used on expressions.
+ */
+class MovableChecker: public SideEffectsCollector
+{
+public:
+	explicit MovableChecker(Dialect const& _dialect): SideEffectsCollector(_dialect) {}
+	MovableChecker(Dialect const& _dialect, Expression const& _expression);
+
+	void operator()(Identifier const& _identifier) override;
+
+	/// Disallow visiting anything apart from Expressions (this throws).
+	void visit(Statement const&) override;
+	using ASTWalker::visit;
+
+	std::set<YulString> const& referencedVariables() const { return m_variableReferences; }
+
+private:
+	/// Which variables the current expression references.
+	std::set<YulString> m_variableReferences;
+};
+
+
 /**
  * Helper class to find "irregular" control flow.
  * This includes termination, break and continue.

libyul/optimiser/SimplificationRules.cpp

@@ -187,7 +187,7 @@ bool Pattern::matches(
 			assertThrow(firstMatch, OptimizerException, "Match set but to null.");
 			return
 				SyntacticallyEqual{}(*firstMatch, _expr) &&
-				MovableChecker(_dialect, _expr).movable();
+				SideEffectsCollector(_dialect, _expr).movable();
 		}
 		else if (m_kind == PatternKind::Any)
 			(*m_matchGroups)[m_matchGroup] = &_expr;

libyul/optimiser/StackCompressor.cpp

@@ -113,7 +113,12 @@ public:
 };
 
 template <typename ASTNode>
-void eliminateVariables(Dialect const& _dialect, ASTNode& _node, size_t _numVariables)
+void eliminateVariables(
+	Dialect const& _dialect,
+	ASTNode& _node,
+	size_t _numVariables,
+	bool _allowMSizeOptimization
+)
 {
 	RematCandidateSelector selector{_dialect};
 	selector(_node);
@@ -143,7 +148,7 @@ void eliminateVariables(Dialect const& _dialect, ASTNode& _node, size_t _numVari
 	}
 
 	Rematerialiser::run(_dialect, _node, std::move(varsToEliminate));
-	UnusedPruner::runUntilStabilised(_dialect, _node);
+	UnusedPruner::runUntilStabilised(_dialect, _node, _allowMSizeOptimization);
 }
 
 }
@@ -159,6 +164,7 @@ bool StackCompressor::run(
 		_ast.statements.size() > 0 && _ast.statements.at(0).type() == typeid(Block),
 		"Need to run the function grouper before the stack compressor."
 	);
+	bool allowMSizeOptimzation = !SideEffectsCollector(_dialect, _ast).containsMSize();
 	for (size_t iterations = 0; iterations < _maxIterations; iterations++)
 	{
 		map<YulString, int> stackSurplus = CompilabilityChecker::run(_dialect, _ast, _optimizeStackAllocation);
@@ -168,7 +174,12 @@ bool StackCompressor::run(
 		if (stackSurplus.count(YulString{}))
 		{
 			yulAssert(stackSurplus.at({}) > 0, "Invalid surplus value.");
-			eliminateVariables(_dialect, boost::get<Block>(_ast.statements.at(0)), stackSurplus.at({}));
+			eliminateVariables(
+				_dialect,
+				boost::get<Block>(_ast.statements.at(0)),
+				stackSurplus.at({}),
+				allowMSizeOptimzation
+			);
 		}
 
 		for (size_t i = 1; i < _ast.statements.size(); ++i)
@@ -178,7 +189,12 @@ bool StackCompressor::run(
 				continue;
 
 			yulAssert(stackSurplus.at(fun.name) > 0, "Invalid surplus value.");
-			eliminateVariables(_dialect, fun, stackSurplus.at(fun.name));
+			eliminateVariables(
+				_dialect,
+				fun,
+				stackSurplus.at(fun.name),
+				allowMSizeOptimzation
+			);
 		}
 	}
 	return false;

libyul/optimiser/UnusedPruner.cpp

@@ -32,16 +32,28 @@ using namespace std;
 using namespace dev;
 using namespace yul;
 
-UnusedPruner::UnusedPruner(Dialect const& _dialect, Block& _ast, set<YulString> const& _externallyUsedFunctions):
-	m_dialect(_dialect)
+UnusedPruner::UnusedPruner(
+	Dialect const& _dialect,
+	Block& _ast,
+	bool _allowMSizeOptimization,
+	set<YulString> const& _externallyUsedFunctions
+):
+	m_dialect(_dialect),
+	m_allowMSizeOptimization(_allowMSizeOptimization)
 {
 	m_references = ReferencesCounter::countReferences(_ast);
 	for (auto const& f: _externallyUsedFunctions)
 		++m_references[f];
 }
 
-UnusedPruner::UnusedPruner(Dialect const& _dialect, FunctionDefinition& _function, set<YulString> const& _externallyUsedFunctions):
-	m_dialect(_dialect)
+UnusedPruner::UnusedPruner(
+	Dialect const& _dialect,
+	FunctionDefinition& _function,
+	bool _allowMSizeOptimization,
+	set<YulString> const& _externallyUsedFunctions
+):
+	m_dialect(_dialect),
+	m_allowMSizeOptimization(_allowMSizeOptimization)
 {
 	m_references = ReferencesCounter::countReferences(_function);
 	for (auto const& f: _externallyUsedFunctions)
@@ -75,7 +87,7 @@ void UnusedPruner::operator()(Block& _block)
 			{
 				if (!varDecl.value)
 					statement = Block{std::move(varDecl.location), {}};
-				else if (MovableChecker(m_dialect, *varDecl.value).sideEffectFree())
+				else if (SideEffectsCollector(m_dialect, *varDecl.value).sideEffectFree(m_allowMSizeOptimization))
 				{
 					subtractReferences(ReferencesCounter::countReferences(*varDecl.value));
 					statement = Block{std::move(varDecl.location), {}};
@@ -93,7 +105,7 @@ void UnusedPruner::operator()(Block& _block)
 		else if (statement.type() == typeid(ExpressionStatement))
 		{
 			ExpressionStatement& exprStmt = boost::get<ExpressionStatement>(statement);
-			if (MovableChecker(m_dialect, exprStmt.expression).sideEffectFree())
+			if (SideEffectsCollector(m_dialect, exprStmt.expression).sideEffectFree(m_allowMSizeOptimization))
 			{
 				subtractReferences(ReferencesCounter::countReferences(exprStmt.expression));
 				statement = Block{std::move(exprStmt.location), {}};
@@ -108,27 +120,41 @@ void UnusedPruner::operator()(Block& _block)
 void UnusedPruner::runUntilStabilised(
 	Dialect const& _dialect,
 	Block& _ast,
+	bool _allowMSizeOptization,
 	set<YulString> const& _externallyUsedFunctions
 )
 {
+	_allowMSizeOptization = !SideEffectsCollector(_dialect, _ast).containsMSize();
+
 	while (true)
 	{
-		UnusedPruner pruner(_dialect, _ast, _externallyUsedFunctions);
+		UnusedPruner pruner(_dialect, _ast, _allowMSizeOptization, _externallyUsedFunctions);
 		pruner(_ast);
 		if (!pruner.shouldRunAgain())
 			return;
 	}
 }
 
+void UnusedPruner::runUntilStabilised(
+	Dialect const& _dialect,
+	Block& _ast,
+	set<YulString> const& _externallyUsedFunctions
+)
+{
+	bool allowMSizeOptimization = !SideEffectsCollector(_dialect, _ast).containsMSize();
+	runUntilStabilised(_dialect, _ast, allowMSizeOptimization, _externallyUsedFunctions);
+}
+
 void UnusedPruner::runUntilStabilised(
 	Dialect const& _dialect,
 	FunctionDefinition& _function,
+	bool _allowMSizeOptimization,
 	set<YulString> const& _externallyUsedFunctions
 )
 {
 	while (true)
 	{
-		UnusedPruner pruner(_dialect, _function, _externallyUsedFunctions);
+		UnusedPruner pruner(_dialect, _function, _allowMSizeOptimization, _externallyUsedFunctions);
 		pruner(_function);
 		if (!pruner.shouldRunAgain())
 			return;

libyul/optimiser/UnusedPruner.h

@@ -44,11 +44,13 @@ public:
 	UnusedPruner(
 		Dialect const& _dialect,
 		Block& _ast,
+		bool _allowMSizeOptimization,
 		std::set<YulString> const& _externallyUsedFunctions = {}
 	);
 	UnusedPruner(
 		Dialect const& _dialect,
 		FunctionDefinition& _function,
+		bool _allowMSizeOptimization,
 		std::set<YulString> const& _externallyUsedFunctions = {}
 	);
 
@@ -59,6 +61,13 @@ public:
 	bool shouldRunAgain() const { return m_shouldRunAgain; }
 
 	// Run the pruner until the code does not change anymore.
+	static void runUntilStabilised(
+		Dialect const& _dialect,
+		Block& _ast,
+		bool _allowMSizeOptization,
+		std::set<YulString> const& _externallyUsedFunctions = {}
+	);
+
 	static void runUntilStabilised(
 		Dialect const& _dialect,
 		Block& _ast,
@@ -67,9 +76,13 @@ public:
 
 	// Run the pruner until the code does not change anymore.
 	// Only run on the given function.
+	// @param _allowMSizeOptimization if true, allows to remove instructions
+	//        whose only side-effect is a potential change of the return value of
+	//        the msize instruction.
 	static void runUntilStabilised(
 		Dialect const& _dialect,
 		FunctionDefinition& _functionDefinition,
+		bool _allowMSizeOptimization,
 		std::set<YulString> const& _externallyUsedFunctions = {}
 	);
 
@@ -78,6 +91,7 @@ private:
 	void subtractReferences(std::map<YulString, size_t> const& _subtrahend);
 
 	Dialect const& m_dialect;
+	bool m_allowMSizeOptimization = false;
 	bool m_shouldRunAgain = false;
 	std::map<YulString, size_t> m_references;
 };