more work

libyul/optimiser/UnusedAssignEliminator.cpp

@@ -45,17 +45,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast)
 	UnusedAssignEliminator rae{_context.dialect};
 	rae(_ast);
 
-	set<Statement const*> toRemove;
+	rae.m_storesToRemove += move(rae.m_potentiallyUnusedStores);
-	for (Statement const* unusedStore: rae.m_allStores - rae.m_usedStores)
-		// TODO this should also use user function side effects.
-		// Then we have to modify the multi-assign test (or verify that it is fine after all
-		// by adding a test where one var is used but not the other)
-		if (SideEffectsCollector{_context.dialect, *std::get<Assignment>(*unusedStore).value}.movable())
-			toRemove.insert(unusedStore);
-		else
-			cerr << "not used because not movable" << endl;
 
-	StatementRemover remover{toRemove};
+	StatementRemover remover{rae.m_storesToRemove};
 	remover(_ast);
 }
 
@@ -64,14 +56,6 @@ void UnusedAssignEliminator::operator()(Identifier const& _identifier)
 	markUsed(_identifier.name);
 }
 
-void UnusedAssignEliminator::operator()(VariableDeclaration const& _variableDeclaration)
-{
-	UnusedStoreBase::operator()(_variableDeclaration);
-
-	for (auto const& var: _variableDeclaration.variables)
-		m_declaredVariables.emplace(var.name);
-}
-
 void UnusedAssignEliminator::operator()(Assignment const& _assignment)
 {
 	visit(*_assignment.value);
@@ -81,7 +65,6 @@ void UnusedAssignEliminator::operator()(Assignment const& _assignment)
 
 void UnusedAssignEliminator::operator()(FunctionDefinition const& _functionDefinition)
 {
-	ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {});
 	ScopedSaveAndRestore outerReturnVariables(m_returnVariables, {});
 
 	for (auto const& retParam: _functionDefinition.returnVariables)
@@ -98,10 +81,9 @@ void UnusedAssignEliminator::operator()(Leave const&)
 
 void UnusedAssignEliminator::operator()(Block const& _block)
 {
-	ScopedSaveAndRestore outerDeclaredVariables(m_declaredVariables, {});
-
 	UnusedStoreBase::operator()(_block);
 
+	// TODO we could also move some statements from "potentially" to "toRemove".
 	for (auto const& statement: _block.statements)
 		if (auto const* varDecl = get_if<VariableDeclaration>(&statement))
 			for (auto const& var: varDecl->variables)
@@ -114,9 +96,18 @@ void UnusedAssignEliminator::visit(Statement const& _statement)
 
 	if (auto const* assignment = get_if<Assignment>(&_statement))
 	{
-		m_allStores.insert(&_statement);
+		// TODO this should also use user function side effects.
-		for (auto const& var: assignment->variableNames)
+		// Then we have to modify the multi-assign test (or verify that it is fine after all
-			m_activeStores[var.name] = {&_statement};
+		// by adding a test where one var is used but not the other)
+		if (SideEffectsCollector{m_dialect, *assignment->value}.movable())
+		{
+			m_potentiallyUnusedStores.insert(&_statement);
+			for (auto const& var: assignment->variableNames)
+				m_activeStores[var.name] = {&_statement};
+		}
+		else
+			for (auto const& var: assignment->variableNames)
+				m_activeStores[var.name].clear();
 	}
 
 //	cerr << "After " << std::visit(AsmPrinter{}, _statement) << endl;
@@ -144,7 +135,7 @@ void UnusedAssignEliminator::shortcutNestedLoop(ActiveStores const& _zeroRuns)
 			auto zeroIt = _zeroRuns.find(variable);
 			if (zeroIt != _zeroRuns.end() && zeroIt->second.count(assignment))
 				continue;
-			m_usedStores.insert(assignment);
+			m_potentiallyUnusedStores.erase(assignment);
 		}
 }
 
@@ -157,7 +148,6 @@ void UnusedAssignEliminator::finalizeFunctionDefinition(FunctionDefinition const
 void UnusedAssignEliminator::markUsed(YulString _variable)
 {
 	for (auto& assignment: m_activeStores[_variable])
-		m_usedStores.insert(assignment);
+		m_potentiallyUnusedStores.erase(assignment);
-	// TODO is this correct?
 	m_activeStores.erase(_variable);
 }

libyul/optimiser/UnusedAssignEliminator.h

@@ -116,7 +116,6 @@ public:
 	explicit UnusedAssignEliminator(Dialect const& _dialect): UnusedStoreBase(_dialect) {}
 
 	void operator()(Identifier const& _identifier) override;
-	void operator()(VariableDeclaration const& _variableDeclaration) override;
 	void operator()(Assignment const& _assignment) override;
 	void operator()(FunctionDefinition const&) override;
 	void operator()(Leave const&) override;
@@ -131,7 +130,6 @@ private:
 
 	void markUsed(YulString _variable);
 
-	std::set<YulString> m_declaredVariables;
 	std::set<YulString> m_returnVariables;
 };
 

libyul/optimiser/UnusedStoreBase.cpp

@@ -74,10 +74,12 @@ void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition)
 	ScopedSaveAndRestore outerAssignments(m_activeStores, {});
 	ScopedSaveAndRestore forLoopInfo(m_forLoopInfo, {});
 	ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, 0);
+	ScopedSaveAndRestore potentiallyUnused(m_potentiallyUnusedStores, {});
 
 	(*this)(_functionDefinition.body);
 
 	finalizeFunctionDefinition(_functionDefinition);
+	m_storesToRemove += move(m_potentiallyUnusedStores);
 }
 
 void UnusedStoreBase::operator()(ForLoop const& _forLoop)

libyul/optimiser/UnusedStoreBase.h

@@ -74,10 +74,11 @@ protected:
 	static void merge(ActiveStores& _target, std::vector<ActiveStores>&& _source);
 
 	Dialect const& m_dialect;
-	/// Set of all stores encountered during the traversal
+	/// Set of stores that unused. Once a store is deemed used, it is removed from here.
-	std::set<Statement const*> m_allStores;
+	std::set<Statement const*> m_potentiallyUnusedStores;
-	/// Set of stores that are marked as being used.
+	/// Statements are moved from m_potentiallUnusedStores to m_storesToRemove at the
-	std::set<Statement const*> m_usedStores;
+	/// end of each function.
+	std::set<Statement const*> m_storesToRemove;
 	/// Active (undecided) stores in the current branch.
 	ActiveStores m_activeStores;
 

libyul/optimiser/UnusedStoreEliminator.cpp

@@ -85,8 +85,9 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast)
 	else
 		rse.markActiveAsUsed(Location::Memory);
 	rse.markActiveAsUsed(Location::Storage);
+	rse.m_storesToRemove += move(rse.m_potentiallyUnusedStores);
 
-	StatementRemover remover{rse.m_allStores - rse.m_usedStores};
+	StatementRemover remover{rse.m_storesToRemove};
 	remover(_ast);
 }
 
@@ -190,7 +191,7 @@ void UnusedStoreEliminator::visit(Statement const& _statement)
 			if (!allowReturndatacopyToBeRemoved)
 				return;
 		}
-		m_allStores.insert(&_statement);
+		m_potentiallyUnusedStores.insert(&_statement);
 		vector<Operation> operations = operationsFromFunctionCall(*funCall);
 		yulAssert(operations.size() == 1, "");
 		if (operations.front().location == Location::Storage)
@@ -267,7 +268,7 @@ void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation cons
 		if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation))
 		{
 			// This store is read from, mark it as used and remove it from the active set.
-			m_usedStores.insert(statement);
+			m_potentiallyUnusedStores.erase(statement);
 			it = active.erase(it);
 		}
 		else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation))
@@ -402,10 +403,10 @@ void UnusedStoreEliminator::markActiveAsUsed(
 {
 	if (_onlyLocation == nullopt || _onlyLocation == Location::Memory)
 		for (Statement const* statement: activeMemoryStores())
-			m_usedStores.insert(statement);
+			m_potentiallyUnusedStores.erase(statement);
 	if (_onlyLocation == nullopt || _onlyLocation == Location::Storage)
 		for (Statement const* statement: activeStorageStores())
-			m_usedStores.insert(statement);
+			m_potentiallyUnusedStores.erase(statement);
 	clearActive(_onlyLocation);
 }