Merge pull request #6746 from ethereum/splitMovable

Split movable property into movable and side-effect-free

libevmasm/SemanticInformation.cpp

@@ -208,6 +208,27 @@ bool SemanticInformation::movable(Instruction _instruction)
 	return true;
 }
 
+bool SemanticInformation::sideEffectFree(Instruction _instruction)
+{
+	// These are not really functional.
+	assertThrow(!isDupInstruction(_instruction) && !isSwapInstruction(_instruction), AssemblyException, "");
+
+	InstructionInfo info = instructionInfo(_instruction);
+	switch (_instruction)
+	{
+	// All the instructions that merely read memory are fine
+	// even though they are marked "sideEffects" in Instructions.cpp
+	case Instruction::KECCAK256:
+	case Instruction::MLOAD:
+		return true;
+	default:
+		break;
+	}
+	if (info.sideEffects)
+		return false;
+	return true;
+}
+
 bool SemanticInformation::invalidatesMemory(Instruction _instruction)
 {
 	switch (_instruction)

libevmasm/SemanticInformation.h

@@ -56,6 +56,10 @@ struct SemanticInformation
 	/// without altering the semantics. This means it cannot depend on storage or memory,
 	/// cannot have any side-effects, but it can depend on a call-constant state of the blockchain.
 	static bool movable(Instruction _instruction);
+	/// @returns true if the instruction can be removed without changing the semantics.
+	/// This does not mean that it has to be deterministic or retrieve information from
+	/// somewhere else than purely the values of its arguments.
+	static bool sideEffectFree(Instruction _instruction);
 	/// @returns true if the given instruction modifies memory.
 	static bool invalidatesMemory(Instruction _instruction);
 	/// @returns true if the given instruction modifies storage (even indirectly).

libyul/Dialect.h

@@ -49,6 +49,8 @@ struct BuiltinFunction
 	/// This means the function cannot depend on storage or memory, cannot have any side-effects,
 	/// but it can depend on state that is constant across an EVM-call.
 	bool movable = false;
+	/// If true, a call to this function can be omitted without changing semantics.
+	bool sideEffectFree = false;
 	/// If true, can only accept literals as arguments and they cannot be moved to variables.
 	bool literalArguments = false;
 };

libyul/backends/evm/EVMDialect.cpp

@@ -42,7 +42,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess, langutil::EVMVer
 	if (!m_objectAccess)
 		return;
 
-	addFunction("datasize", 1, 1, true, true, [this](
+	addFunction("datasize", 1, 1, true, true, true, [this](
 		FunctionCall const& _call,
 		AbstractAssembly& _assembly,
 		std::function<void()>
@@ -59,7 +59,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess, langutil::EVMVer
 			_assembly.appendDataSize(m_subIDs.at(dataName));
 		}
 	});
-	addFunction("dataoffset", 1, 1, true, true, [this](
+	addFunction("dataoffset", 1, 1, true, true, true, [this](
 		FunctionCall const& _call,
 		AbstractAssembly& _assembly,
 		std::function<void()>
@@ -76,7 +76,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess, langutil::EVMVer
 			_assembly.appendDataOffset(m_subIDs.at(dataName));
 		}
 	});
-	addFunction("datacopy", 3, 0, false, false, [](
+	addFunction("datacopy", 3, 0, false, false, false, [](
 		FunctionCall const&,
 		AbstractAssembly& _assembly,
 		std::function<void()> _visitArguments
@@ -132,6 +132,7 @@ void EVMDialect::addFunction(
 	size_t _params,
 	size_t _returns,
 	bool _movable,
+	bool _sideEffectFree,
 	bool _literalArguments,
 	std::function<void(FunctionCall const&, AbstractAssembly&, std::function<void()>)> _generateCode
 )
@@ -142,6 +143,7 @@ void EVMDialect::addFunction(
 	f.parameters.resize(_params);
 	f.returns.resize(_returns);
 	f.movable = _movable;
+	f.sideEffectFree = _sideEffectFree;
 	f.literalArguments = _literalArguments;
 	f.generateCode = std::move(_generateCode);
 }

libyul/backends/evm/EVMDialect.h

@@ -75,6 +75,7 @@ protected:
 		size_t _params,
 		size_t _returns,
 		bool _movable,
+		bool _sideEffectFree,
 		bool _literalArguments,
 		std::function<void(FunctionCall const&, AbstractAssembly&, std::function<void()>)> _generateCode
 	);

libyul/backends/wasm/WasmDialect.cpp

@@ -71,5 +71,6 @@ void WasmDialect::addFunction(string _name, size_t _params, size_t _returns)
 	f.parameters.resize(_params);
 	f.returns.resize(_returns);
 	f.movable = false;
+	f.sideEffectFree = false;
 	f.literalArguments = false;
 }

libyul/optimiser/Semantics.cpp

@@ -51,21 +51,30 @@ void MovableChecker::operator()(Identifier const& _identifier)
 
 void MovableChecker::operator()(FunctionalInstruction const& _instr)
 {
+	ASTWalker::operator()(_instr);
+
 	if (!eth::SemanticInformation::movable(_instr.instruction))
 		m_movable = false;
-	else
+	if (!eth::SemanticInformation::sideEffectFree(_instr.instruction))
-		ASTWalker::operator()(_instr);
+		m_sideEffectFree = false;
 }
 
 void MovableChecker::operator()(FunctionCall const& _functionCall)
-{
-	if (BuiltinFunction const* f = m_dialect.builtin(_functionCall.functionName.name))
-		if (f->movable)
 {
 	ASTWalker::operator()(_functionCall);
-			return;
+
-		}
+	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&)

libyul/optimiser/Semantics.h

@@ -46,6 +46,8 @@ public:
 	using ASTWalker::visit;
 
 	bool movable() const { return m_movable; }
+	bool sideEffectFree() const { return m_sideEffectFree; }
+
 	std::set<YulString> const& referencedVariables() const { return m_variableReferences; }
 
 private:
@@ -54,6 +56,9 @@ private:
 	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;
 };
 
 /**

libyul/optimiser/UnusedPruner.cpp

@@ -75,7 +75,7 @@ void UnusedPruner::operator()(Block& _block)
 			{
 				if (!varDecl.value)
 					statement = Block{std::move(varDecl.location), {}};
-				else if (MovableChecker(m_dialect, *varDecl.value).movable())
+				else if (MovableChecker(m_dialect, *varDecl.value).sideEffectFree())
 				{
 					subtractReferences(ReferencesCounter::countReferences(*varDecl.value));
 					statement = Block{std::move(varDecl.location), {}};
@@ -93,9 +93,8 @@ void UnusedPruner::operator()(Block& _block)
 		else if (statement.type() == typeid(ExpressionStatement))
 		{
 			ExpressionStatement& exprStmt = boost::get<ExpressionStatement>(statement);
-			if (MovableChecker(m_dialect, exprStmt.expression).movable())
+			if (MovableChecker(m_dialect, exprStmt.expression).sideEffectFree())
 			{
-				// pop(x) should be movable!
 				subtractReferences(ReferencesCounter::countReferences(exprStmt.expression));
 				statement = Block{std::move(exprStmt.location), {}};
 			}

libyul/optimiser/UnusedPruner.h

@@ -32,7 +32,7 @@ struct Dialect;
 
 /**
  * Optimisation stage that removes unused variables and functions and also
- * removes movable expression statements.
+ * removes side-effect-free expression statements.
  *
  * Note that this does not remove circular references.
  *

test/libyul/yulOptimizerTests/fullSimplify/identity_rules_simple.yul

@@ -5,8 +5,4 @@
 // ====
 // step: fullSimplify
 // ----
-// {
+// { mstore(0, 0) }
-//     let _1 := 0
-//     pop(mload(_1))
-//     mstore(_1, 0)
-// }

test/libyul/yulOptimizerTests/fullSimplify/not_applied_removes_non_constant_and_not_movable.yul

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

test/libyul/yulOptimizerTests/fullSimplify/operations.yul

@@ -23,7 +23,6 @@
 // step: fullSimplify
 // ----
 // {
-//     pop(mload(0))
 //     mstore(1, 0)
 //     mstore(2, 0)
 //     mstore(3, 0)

test/libyul/yulOptimizerTests/ssaAndBack/for_loop.yul

@@ -24,8 +24,6 @@
 //     for { } lt(mload(a), mload(b)) { a := mload(b) }
 //     {
 //         let b_3 := mload(a)
-//         pop(mload(b_3))
-//         pop(mload(b_3))
 //         let a_6 := mload(b_3)
 //         b := mload(a_6)
 //     }

test/libyul/yulOptimizerTests/ssaAndBack/multi_assign.yul

@@ -10,10 +10,6 @@
 // step: ssaAndBack
 // ----
 // {
-//     pop(mload(0))
-//     pop(mload(1))
-//     pop(mload(2))
-//     pop(mload(3))
 //     let a_5 := mload(4)
 //     mstore(a_5, 0)
 // }

test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_if.yul

@@ -13,11 +13,6 @@
 // ----
 // {
 //     let a := mload(0)
-//     if mload(1)
+//     if mload(1) { a := mload(3) }
-//     {
-//         pop(mload(1))
-//         pop(mload(2))
-//         a := mload(3)
-//     }
 //     mstore(a, 0)
 // }

test/libyul/yulOptimizerTests/ssaAndBack/multi_assign_switch.yul

@@ -19,15 +19,7 @@
 // {
 //     let a := mload(0)
 //     switch mload(1)
-//     case 0 {
+//     case 0 { a := mload(3) }
-//         pop(mload(1))
+//     default { a := mload(6) }
-//         pop(mload(2))
-//         a := mload(3)
-//     }
-//     default {
-//         pop(mload(4))
-//         pop(mload(5))
-//         a := mload(6)
-//     }
 //     mstore(a, 0)
 // }

test/libyul/yulOptimizerTests/ssaAndBack/simple.yul

@@ -7,7 +7,6 @@
 // step: ssaAndBack
 // ----
 // {
-//     pop(mload(0))
 //     let a_2 := mload(1)
 //     mstore(a_2, 0)
 // }