Refactor SideEffects struct

libyul/SideEffects.h

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <algorithm>
 #include <set>
 
 namespace solidity::yul
@@ -30,6 +31,20 @@ namespace solidity::yul
  */
 struct SideEffects
 {
+	/// Corresponds to the effect that a Yul-builtin has on a generic data location (storage, memory
+	/// and other blockchain state).
+	enum Effect
+	{
+		None,
+		Read,
+		Write
+	};
+
+	friend Effect operator+(Effect const& _a, Effect const& _b)
+	{
+		return static_cast<Effect>(std::max(static_cast<int>(_a), static_cast<int>(_b)));
+	}
+
 	/// If true, expressions in this code can be freely moved and copied without altering the
 	/// semantics.
 	/// At statement level, it means that functions containing this code can be
@@ -38,22 +53,34 @@ struct SideEffects
 	/// This means it 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 = true;
+	/// If true, the expressions in this code can be moved or copied (together with their arguments)
+	/// across control flow branches and instructions as long as these instructions' 'effects' do
+	/// not influence the 'effects' of the aforementioned expressions.
+	bool movableApartFromEffects = true;
 	/// If true, the code can be removed without changing the semantics.
-	bool sideEffectFree = true;
+	bool canBeRemoved = true;
 	/// If true, the code can be removed without changing the semantics as long as
 	/// the whole program does not contain the msize instruction.
-	bool sideEffectFreeIfNoMSize = true;
-	/// If false, storage is guaranteed to be unchanged by the code under all
-	/// circumstances.
-	bool invalidatesStorage = false;
-	/// If false, memory is guaranteed to be unchanged by the code under all
-	/// circumstances.
-	bool invalidatesMemory = false;
+	bool canBeRemovedIfNoMSize = true;
+	/// If false, the code calls a for-loop or a recursive function, and therefore potentially loops
+	/// infinitely. All builtins are set to true by default, even `invalid()`.
+	bool cannotLoop = true;
+	/// Can write, read or have no effect on the blockchain state, when the value of `otherState` is
+	/// `Write`, `Read` or `None` respectively.
+	Effect otherState = None;
+	/// Can write, read or have no effect on storage, when the value of `storage` is `Write`, `Read`
+	/// or `None` respectively. When the value is `Write`, the expression can invalidate storage,
+	/// potentially indirectly through external calls.
+	Effect storage = None;
+	/// Can write, read or have no effect on memory, when the value of `memory` is `Write`, `Read`
+	/// or `None` respectively. Note that, when the value is `Read`, the expression can have an
+	/// effect on `msize()`.
+	Effect memory = None;
 
 	/// @returns the worst-case side effects.
 	static SideEffects worst()
 	{
-		return SideEffects{false, false, false, true, true};
+		return SideEffects{false, false, false, false, false, Write, Write, Write};
 	}
 
 	/// @returns the combined side effects of two pieces of code.
@@ -61,10 +88,13 @@ struct SideEffects
 	{
 		return SideEffects{
 			movable && _other.movable,
-			sideEffectFree && _other.sideEffectFree,
-			sideEffectFreeIfNoMSize && _other.sideEffectFreeIfNoMSize,
-			invalidatesStorage || _other.invalidatesStorage,
-			invalidatesMemory || _other.invalidatesMemory
+			movableApartFromEffects && _other.movableApartFromEffects,
+			canBeRemoved && _other.canBeRemoved,
+			canBeRemovedIfNoMSize && _other.canBeRemovedIfNoMSize,
+			cannotLoop && _other.cannotLoop,
+			otherState + _other.otherState,
+			storage + _other.storage,
+			memory +  _other.memory
 		};
 	}
 
@@ -79,10 +109,13 @@ struct SideEffects
 	{
 		return
 			movable == _other.movable &&
-			sideEffectFree == _other.sideEffectFree &&
-			sideEffectFreeIfNoMSize == _other.sideEffectFreeIfNoMSize &&
-			invalidatesStorage == _other.invalidatesStorage &&
-			invalidatesMemory == _other.invalidatesMemory;
+			movableApartFromEffects == _other.movableApartFromEffects &&
+			canBeRemoved == _other.canBeRemoved &&
+			canBeRemovedIfNoMSize == _other.canBeRemovedIfNoMSize &&
+			cannotLoop == _other.cannotLoop &&
+			otherState == _other.otherState &&
+			storage == _other.storage &&
+			memory == _other.memory;
 	}
 };
 

libyul/backends/evm/EVMDialect.cpp

@@ -190,7 +190,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 			"datacopy",
 			3,
 			0,
-			SideEffects{false, false, false, false, true},
+			SideEffects{false, true, false, false, true, SideEffects::None, SideEffects::None, SideEffects::Write},
 			{},
 			[](
 				FunctionCall const& _call,
@@ -206,7 +206,7 @@ map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVe
 			"setimmutable",
 			2,
 			0,
-			SideEffects{false, false, false, false, true},
+			SideEffects{false, false, false, false, true, SideEffects::None, SideEffects::None, SideEffects::Write},
 			{LiteralKind::String, std::nullopt},
 			[](
 				FunctionCall const& _call,
@@ -281,12 +281,20 @@ EVMDialect const& EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion _
 
 SideEffects EVMDialect::sideEffectsOfInstruction(evmasm::Instruction _instruction)
 {
+	auto translate = [](evmasm::SemanticInformation::Effect _e) -> SideEffects::Effect
+	{
+		return static_cast<SideEffects::Effect>(_e);
+	};
+
 	return SideEffects{
 		evmasm::SemanticInformation::movable(_instruction),
-		evmasm::SemanticInformation::sideEffectFree(_instruction),
-		evmasm::SemanticInformation::sideEffectFreeIfNoMSize(_instruction),
-		evmasm::SemanticInformation::invalidatesStorage(_instruction),
-		evmasm::SemanticInformation::invalidatesMemory(_instruction)
+		evmasm::SemanticInformation::movableApartFromEffects(_instruction),
+		evmasm::SemanticInformation::canBeRemoved(_instruction),
+		evmasm::SemanticInformation::canBeRemovedIfNoMSize(_instruction),
+		true, // cannotLoop
+		translate(evmasm::SemanticInformation::otherState(_instruction)),
+		translate(evmasm::SemanticInformation::storage(_instruction)),
+		translate(evmasm::SemanticInformation::memory(_instruction)),
 	};
 }
 

libyul/backends/wasm/WasmDialect.cpp

@@ -86,27 +86,34 @@ WasmDialect::WasmDialect()
 	addFunction("i64.extend_i32_u", {i32}, {i64});
 
 	addFunction("i32.store", {i32, i32}, {}, false);
-	m_functions["i32.store"_yulstring].sideEffects.invalidatesStorage = false;
+	m_functions["i32.store"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["i32.store"_yulstring].sideEffects.otherState = SideEffects::None;
 	addFunction("i64.store", {i32, i64}, {}, false);
-	m_functions["i64.store"_yulstring].sideEffects.invalidatesStorage = false;
 	// TODO: add i32.store16, i64.store8, i64.store16, i64.store32
+	m_functions["i64.store"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["i64.store"_yulstring].sideEffects.otherState = SideEffects::None;
 
 	addFunction("i32.store8", {i32, i32}, {}, false);
-	m_functions["i32.store8"_yulstring].sideEffects.invalidatesStorage = false;
+	m_functions["i32.store8"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["i32.store8"_yulstring].sideEffects.otherState = SideEffects::None;
+
 	addFunction("i64.store8", {i32, i64}, {}, false);
-	m_functions["i64.store8"_yulstring].sideEffects.invalidatesStorage = false;
+	m_functions["i64.store8"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["i64.store8"_yulstring].sideEffects.otherState = SideEffects::None;
 
 	addFunction("i32.load", {i32}, {i32}, false);
-	m_functions["i32.load"_yulstring].sideEffects.invalidatesStorage = false;
-	m_functions["i32.load"_yulstring].sideEffects.invalidatesMemory = false;
-	m_functions["i32.load"_yulstring].sideEffects.sideEffectFree = true;
-	m_functions["i32.load"_yulstring].sideEffects.sideEffectFreeIfNoMSize = true;
+	m_functions["i32.load"_yulstring].sideEffects.canBeRemoved = true;
+	m_functions["i32.load"_yulstring].sideEffects.canBeRemovedIfNoMSize = true;
+	m_functions["i32.load"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["i32.load"_yulstring].sideEffects.memory = SideEffects::Read;
+	m_functions["i32.load"_yulstring].sideEffects.otherState = SideEffects::None;
 	addFunction("i64.load", {i32}, {i64}, false);
-	m_functions["i64.load"_yulstring].sideEffects.invalidatesStorage = false;
-	m_functions["i64.load"_yulstring].sideEffects.invalidatesMemory = false;
-	m_functions["i64.load"_yulstring].sideEffects.sideEffectFree = true;
-	m_functions["i64.load"_yulstring].sideEffects.sideEffectFreeIfNoMSize = true;
 	// TODO: add i32.load8, i32.load16, i64.load8, i64.load16, i64.load32
+	m_functions["i64.load"_yulstring].sideEffects.canBeRemoved = true;
+	m_functions["i64.load"_yulstring].sideEffects.canBeRemovedIfNoMSize = true;
+	m_functions["i64.load"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["i64.load"_yulstring].sideEffects.memory = SideEffects::Read;
+	m_functions["i64.load"_yulstring].sideEffects.otherState = SideEffects::None;
 
 	// Drop is actually overloaded for all types, but Yul does not support that.
 	// Because of that, we introduce "i32.drop" and "i64.drop".
@@ -115,8 +122,9 @@ WasmDialect::WasmDialect()
 
 	addFunction("nop", {}, {});
 	addFunction("unreachable", {}, {}, false);
-	m_functions["unreachable"_yulstring].sideEffects.invalidatesStorage = false;
-	m_functions["unreachable"_yulstring].sideEffects.invalidatesMemory = false;
+	m_functions["unreachable"_yulstring].sideEffects.storage = SideEffects::None;
+	m_functions["unreachable"_yulstring].sideEffects.memory = SideEffects::None;
+	m_functions["unreachable"_yulstring].sideEffects.otherState = SideEffects::None;
 	m_functions["unreachable"_yulstring].controlFlowSideEffects.terminates = true;
 	m_functions["unreachable"_yulstring].controlFlowSideEffects.reverts = true;
 
@@ -219,10 +227,24 @@ void WasmDialect::addEthereumExternals()
 			f.returns.emplace_back(YulString(p));
 		// TODO some of them are side effect free.
 		f.sideEffects = SideEffects::worst();
+		f.sideEffects.cannotLoop = true;
+		f.sideEffects.movableApartFromEffects = !ext.controlFlowSideEffects.terminates;
 		f.controlFlowSideEffects = ext.controlFlowSideEffects;
 		f.isMSize = false;
-		f.sideEffects.invalidatesStorage = (ext.name == "storageStore");
 		f.literalArguments.clear();
+
+		static set<string> const writesToStorage{
+			"storageStore",
+			"call",
+			"callcode",
+			"callDelegate",
+			"create"
+		};
+		static set<string> const readsStorage{"storageLoad", "callStatic"};
+		if (readsStorage.count(ext.name))
+			f.sideEffects.storage = SideEffects::Read;
+		else if (!writesToStorage.count(ext.name))
+			f.sideEffects.storage = SideEffects::None;
 	}
 }
 
@@ -241,6 +263,9 @@ void WasmDialect::addFunction(
 	yulAssert(_returns.size() <= 1, "The Wasm 1.0 specification only allows up to 1 return value.");
 	f.returns = std::move(_returns);
 	f.sideEffects = _movable ? SideEffects{} : SideEffects::worst();
+	f.sideEffects.cannotLoop = true;
+	// TODO This should be improved when LoopInvariantCodeMotion gets specialized for WASM
+	f.sideEffects.movableApartFromEffects = _movable;
 	f.isMSize = false;
 	f.literalArguments = std::move(_literalArguments);
 }

libyul/optimiser/Semantics.cpp

@@ -106,8 +106,9 @@ map<YulString, SideEffects> SideEffectsPropagator::sideEffects(
 	for (auto const& function: _directCallGraph.functionsWithLoops + _directCallGraph.recursiveFunctions())
 	{
 		ret[function].movable = false;
-		ret[function].sideEffectFree = false;
-		ret[function].sideEffectFreeIfNoMSize = false;
+		ret[function].canBeRemoved = false;
+		ret[function].canBeRemovedIfNoMSize = false;
+		ret[function].cannotLoop = false;
 	}
 
 	for (auto const& call: _directCallGraph.functionCalls)

libyul/optimiser/Semantics.h

@@ -59,16 +59,17 @@ public:
 	void operator()(FunctionCall const& _functionCall) override;
 
 	bool movable() const { return m_sideEffects.movable; }
-	bool sideEffectFree(bool _allowMSizeModification = false) const
+	bool canBeRemoved(bool _allowMSizeModification = false) const
 	{
 		if (_allowMSizeModification)
-			return sideEffectFreeIfNoMSize();
+			return m_sideEffects.canBeRemovedIfNoMSize;
 		else
-			return m_sideEffects.sideEffectFree;
+			return m_sideEffects.canBeRemoved;
 	}
-	bool sideEffectFreeIfNoMSize() const { return m_sideEffects.sideEffectFreeIfNoMSize; }
-	bool invalidatesStorage() const { return m_sideEffects.invalidatesStorage; }
-	bool invalidatesMemory() const { return m_sideEffects.invalidatesMemory; }
+	bool cannotLoop() const { return m_sideEffects.cannotLoop; }
+	bool invalidatesStorage() const { return m_sideEffects.storage == SideEffects::Write; }
+	bool invalidatesMemory() const { return m_sideEffects.memory == SideEffects::Write; }
+
 
 private:
 	Dialect const& m_dialect;

libyul/optimiser/UnusedPruner.cpp

@@ -93,7 +93,7 @@ void UnusedPruner::operator()(Block& _block)
 					statement = Block{std::move(varDecl.location), {}};
 				else if (
 					SideEffectsCollector(m_dialect, *varDecl.value, m_functionSideEffects).
-					sideEffectFree(m_allowMSizeOptimization)
+					canBeRemoved(m_allowMSizeOptimization)
 				)
 				{
 					subtractReferences(ReferencesCounter::countReferences(*varDecl.value));
@@ -112,7 +112,7 @@ void UnusedPruner::operator()(Block& _block)
 			ExpressionStatement& exprStmt = std::get<ExpressionStatement>(statement);
 			if (
 				SideEffectsCollector(m_dialect, exprStmt.expression, m_functionSideEffects).
-				sideEffectFree(m_allowMSizeOptimization)
+				canBeRemoved(m_allowMSizeOptimization)
 			)
 			{
 				subtractReferences(ReferencesCounter::countReferences(exprStmt.expression));