From ddbcea047b9c3aacaa3c172f6e9768cba2aa739e Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 25 Nov 2022 14:20:38 +0100 Subject: [PATCH] some more work --- libyul/optimiser/UnusedAssignEliminator.cpp | 5 +- libyul/optimiser/UnusedStoreEliminator.cpp | 635 +++++++++--------- libyul/optimiser/UnusedStoreEliminator.h | 73 +- .../conditionally_assign_before_revert.yul | 21 + 4 files changed, 395 insertions(+), 339 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul diff --git a/libyul/optimiser/UnusedAssignEliminator.cpp b/libyul/optimiser/UnusedAssignEliminator.cpp index 5da7b8ae6..eaa076fed 100644 --- a/libyul/optimiser/UnusedAssignEliminator.cpp +++ b/libyul/optimiser/UnusedAssignEliminator.cpp @@ -47,6 +47,9 @@ void UnusedAssignEliminator::run(OptimiserStepContext& _context, Block& _ast) set toRemove; 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(*unusedStore).value}.movable()) toRemove.insert(unusedStore); else @@ -111,8 +114,6 @@ void UnusedAssignEliminator::visit(Statement const& _statement) if (auto const* assignment = get_if(&_statement)) { - // TODO is it OK to do this for multi-assignments? I guess so because it is enough if - // one of them is used. m_allStores.insert(&_statement); for (auto const& var: assignment->variableNames) m_activeStores[var.name] = {&_statement}; diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp index aca7dd907..c9b5579c2 100644 --- a/libyul/optimiser/UnusedStoreEliminator.cpp +++ b/libyul/optimiser/UnusedStoreEliminator.cpp @@ -52,123 +52,125 @@ static string const thirtyTwo{"@ 32"}; void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_ast*/) { -// map functionSideEffects = SideEffectsPropagator::sideEffects( -// _context.dialect, -// CallGraphGenerator::callGraph(_ast) -// ); + map functionSideEffects = SideEffectsPropagator::sideEffects( + _context.dialect, + CallGraphGenerator::callGraph(_ast) + ); -// SSAValueTracker ssaValues; -// ssaValues(_ast); -// map values; -// for (auto const& [name, expression]: ssaValues.values()) -// values[name] = AssignedValue{expression, {}}; -// Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; -// Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; -// Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; -// values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; -// values[YulString{one}] = AssignedValue{&oneLiteral, {}}; -// values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; + SSAValueTracker ssaValues; + ssaValues(_ast); + map values; + for (auto const& [name, expression]: ssaValues.values()) + values[name] = AssignedValue{expression, {}}; + Expression const zeroLiteral{Literal{{}, LiteralKind::Number, YulString{"0"}, {}}}; + Expression const oneLiteral{Literal{{}, LiteralKind::Number, YulString{"1"}, {}}}; + Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, YulString{"32"}, {}}}; + values[YulString{zero}] = AssignedValue{&zeroLiteral, {}}; + values[YulString{one}] = AssignedValue{&oneLiteral, {}}; + values[YulString{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}}; -// bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); -// UnusedStoreEliminator rse{ -// _context.dialect, -// functionSideEffects, -// ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), -// values, -// ignoreMemory -// }; -// rse(_ast); -// if ( -// auto evmDialect = dynamic_cast(&_context.dialect); -// evmDialect && evmDialect->providesObjectAccess() -// ) -// rse.changeUndecidedTo(State::Unused, Location::Memory); -// else -// rse.changeUndecidedTo(State::Used, Location::Memory); -// rse.changeUndecidedTo(State::Used, Location::Storage); -// rse.scheduleUnusedForDeletion(); + bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast); + UnusedStoreEliminator rse{ + _context.dialect, + functionSideEffects, + ControlFlowSideEffectsCollector{_context.dialect, _ast}.functionSideEffectsNamed(), + values, + ignoreMemory + }; + rse(_ast); + if ( + auto evmDialect = dynamic_cast(&_context.dialect); + evmDialect && evmDialect->providesObjectAccess() + ) + { + } + else + rse.markActiveAsUsed(Location::Memory); + rse.markActiveAsUsed(Location::Storage); + rse.scheduleUnusedForDeletion(); -// StatementRemover remover(rse.m_pendingRemovals); -// remover(_ast); + StatementRemover remover(rse.m_pendingRemovals); + remover(_ast); } -//void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) -//{ -// UnusedStoreBase::operator()(_functionCall); +void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall) +{ + UnusedStoreBase::operator()(_functionCall); -// for (Operation const& op: operationsFromFunctionCall(_functionCall)) -// applyOperation(op); + for (Operation const& op: operationsFromFunctionCall(_functionCall)) + applyOperation(op); -// ControlFlowSideEffects sideEffects; -// if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) -// sideEffects = builtin->controlFlowSideEffects; -// else -// sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); + ControlFlowSideEffects sideEffects; + if (auto builtin = m_dialect.builtin(_functionCall.functionName.name)) + sideEffects = builtin->controlFlowSideEffects; + else + sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name); -// if (sideEffects.canTerminate) -// changeUndecidedTo(State::Used, Location::Storage); -// if (!sideEffects.canContinue) -// { -// changeUndecidedTo(State::Unused, Location::Memory); -// if (!sideEffects.canTerminate) -// changeUndecidedTo(State::Unused, Location::Storage); -// } -//} + if (sideEffects.canTerminate) + markActiveAsUsed(Location::Storage); + if (!sideEffects.canContinue) + { + clearActive(Location::Memory); + if (!sideEffects.canTerminate) + clearActive(Location::Storage); + } +} -//void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) -//{ -// ScopedSaveAndRestore storeOperations(m_storeOperations, {}); -// UnusedStoreBase::operator()(_functionDefinition); -//} +void UnusedStoreEliminator::operator()(FunctionDefinition const& _functionDefinition) +{ + ScopedSaveAndRestore storeOperations(m_storeOperations, {}); + UnusedStoreBase::operator()(_functionDefinition); +} -//void UnusedStoreEliminator::operator()(Leave const&) -//{ -// changeUndecidedTo(State::Used); -//} +void UnusedStoreEliminator::operator()(Leave const&) +{ + markActiveAsUsed(); +} -//void UnusedStoreEliminator::visit(Statement const& _statement) -//{ -// using evmasm::Instruction; +void UnusedStoreEliminator::visit(Statement const& _statement) +{ + using evmasm::Instruction; -// UnusedStoreBase::visit(_statement); + UnusedStoreBase::visit(_statement); -// auto const* exprStatement = get_if(&_statement); -// if (!exprStatement) -// return; + auto const* exprStatement = get_if(&_statement); + if (!exprStatement) + return; -// FunctionCall const* funCall = get_if(&exprStatement->expression); -// yulAssert(funCall); -// optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); -// if (!instruction) -// return; + FunctionCall const* funCall = get_if(&exprStatement->expression); + yulAssert(funCall); + optional instruction = toEVMInstruction(m_dialect, funCall->functionName.name); + if (!instruction) + return; -// if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { -// return get_if(&_expr) || get_if(&_expr); -// })) -// return; + if (!ranges::all_of(funCall->arguments, [](Expression const& _expr) -> bool { + return get_if(&_expr) || get_if(&_expr); + })) + return; -// // We determine if this is a store instruction without additional side-effects -// // both by querying a combination of semantic information and by listing the instructions. -// // This way the assert below should be triggered on any change. -// using evmasm::SemanticInformation; -// bool isStorageWrite = (*instruction == Instruction::SSTORE); -// bool isMemoryWrite = -// *instruction == Instruction::EXTCODECOPY || -// *instruction == Instruction::CODECOPY || -// *instruction == Instruction::CALLDATACOPY || -// *instruction == Instruction::RETURNDATACOPY || -// *instruction == Instruction::MSTORE || -// *instruction == Instruction::MSTORE8; -// bool isCandidateForRemoval = -// SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( -// SemanticInformation::storage(*instruction) == SemanticInformation::Write || -// (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) -// ); -// yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); -// if (isCandidateForRemoval) -// { -// State initialState = State::Undecided; + // We determine if this is a store instruction without additional side-effects + // both by querying a combination of semantic information and by listing the instructions. + // This way the assert below should be triggered on any change. + using evmasm::SemanticInformation; + bool isStorageWrite = (*instruction == Instruction::SSTORE); + bool isMemoryWrite = + *instruction == Instruction::EXTCODECOPY || + *instruction == Instruction::CODECOPY || + *instruction == Instruction::CALLDATACOPY || + *instruction == Instruction::RETURNDATACOPY || + *instruction == Instruction::MSTORE || + *instruction == Instruction::MSTORE8; + bool isCandidateForRemoval = + SemanticInformation::otherState(*instruction) != SemanticInformation::Write && ( + SemanticInformation::storage(*instruction) == SemanticInformation::Write || + (!m_ignoreMemory && SemanticInformation::memory(*instruction) == SemanticInformation::Write) + ); + yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite))); + if (isCandidateForRemoval) + { + // TODO what is this special case? + //State initialState = State::Undecided; // if (*instruction == Instruction::RETURNDATACOPY) // { // initialState = State::Used; @@ -186,223 +188,254 @@ void UnusedStoreEliminator::run(OptimiserStepContext& /*_context*/, Block& /*_as // initialState = State::Undecided; // } // } -// m_activeStores[YulString{}].insert({&_statement, initialState}); -// vector operations = operationsFromFunctionCall(*funCall); -// yulAssert(operations.size() == 1, ""); -// m_storeOperations[&_statement] = std::move(operations.front()); -// } -//} + m_allStores.insert(&_statement); + vector operations = operationsFromFunctionCall(*funCall); + yulAssert(operations.size() == 1, ""); + if (operations.front().location == Location::Storage) + m_activeStores["s"_yulstring].insert(&_statement); + else + m_activeStores["m"_yulstring].insert(&_statement); + m_storeOperations[&_statement] = std::move(operations.front()); + } +} -//void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) -//{ -// changeUndecidedTo(State::Used); -// scheduleUnusedForDeletion(); -//} +void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&) +{ + markActiveAsUsed(); + scheduleUnusedForDeletion(); +} -//vector UnusedStoreEliminator::operationsFromFunctionCall( -// FunctionCall const& _functionCall -//) const -//{ -// using evmasm::Instruction; +vector UnusedStoreEliminator::operationsFromFunctionCall( + FunctionCall const& _functionCall +) const +{ + using evmasm::Instruction; -// YulString functionName = _functionCall.functionName.name; -// SideEffects sideEffects; -// if (BuiltinFunction const* f = m_dialect.builtin(functionName)) -// sideEffects = f->sideEffects; -// else -// sideEffects = m_functionSideEffects.at(functionName); + YulString functionName = _functionCall.functionName.name; + SideEffects sideEffects; + if (BuiltinFunction const* f = m_dialect.builtin(functionName)) + sideEffects = f->sideEffects; + else + sideEffects = m_functionSideEffects.at(functionName); -// optional instruction = toEVMInstruction(m_dialect, functionName); -// if (!instruction) -// { -// vector result; -// // Unknown read is worse than unknown write. -// if (sideEffects.memory != SideEffects::Effect::None) -// result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); -// if (sideEffects.storage != SideEffects::Effect::None) -// result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); -// return result; -// } + optional instruction = toEVMInstruction(m_dialect, functionName); + if (!instruction) + { + vector result; + // Unknown read is worse than unknown write. + if (sideEffects.memory != SideEffects::Effect::None) + result.emplace_back(Operation{Location::Memory, Effect::Read, {}, {}}); + if (sideEffects.storage != SideEffects::Effect::None) + result.emplace_back(Operation{Location::Storage, Effect::Read, {}, {}}); + return result; + } -// using evmasm::SemanticInformation; + using evmasm::SemanticInformation; -// return util::applyMap( -// SemanticInformation::readWriteOperations(*instruction), -// [&](SemanticInformation::Operation const& _op) -> Operation -// { -// yulAssert(!(_op.lengthParameter && _op.lengthConstant)); -// yulAssert(_op.effect != Effect::None); -// Operation ourOp{_op.location, _op.effect, {}, {}}; -// if (_op.startParameter) -// ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); -// if (_op.lengthParameter) -// ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); -// if (_op.lengthConstant) -// switch (*_op.lengthConstant) -// { -// case 1: ourOp.length = YulString(one); break; -// case 32: ourOp.length = YulString(thirtyTwo); break; -// default: yulAssert(false); -// } -// return ourOp; -// } -// ); -//} + return util::applyMap( + SemanticInformation::readWriteOperations(*instruction), + [&](SemanticInformation::Operation const& _op) -> Operation + { + yulAssert(!(_op.lengthParameter && _op.lengthConstant)); + yulAssert(_op.effect != Effect::None); + Operation ourOp{_op.location, _op.effect, {}, {}}; + if (_op.startParameter) + ourOp.start = identifierNameIfSSA(_functionCall.arguments.at(*_op.startParameter)); + if (_op.lengthParameter) + ourOp.length = identifierNameIfSSA(_functionCall.arguments.at(*_op.lengthParameter)); + if (_op.lengthConstant) + switch (*_op.lengthConstant) + { + case 1: ourOp.length = YulString(one); break; + case 32: ourOp.length = YulString(thirtyTwo); break; + default: yulAssert(false); + } + return ourOp; + } + ); +} -//void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) -//{ -// for (auto& [statement, state]: m_activeStores[YulString{}]) -// if (state == State::Undecided) -// { -// Operation const& storeOperation = m_storeOperations.at(statement); -// if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) -// state = State::Used; -// else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) -// state = State::Unused; -// } -//} +void UnusedStoreEliminator::applyOperation(UnusedStoreEliminator::Operation const& _operation) +{ + // TODO only one will be relevant, depending on _operation.location + for (Statement const* statement: m_activeStores["s"_yulstring]) + { + Operation const& storeOperation = m_storeOperations.at(statement); + if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) + // TODO remove from active! + m_usedStores.insert(statement); + else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) + { + // TODO remove from active +// state = State::Unused; + } + } + for (Statement const* statement: m_activeStores["m"_yulstring]) + { + Operation const& storeOperation = m_storeOperations.at(statement); + if (_operation.effect == Effect::Read && !knownUnrelated(storeOperation, _operation)) + // TODO remove from active! + m_usedStores.insert(statement); + else if (_operation.effect == Effect::Write && knownCovered(storeOperation, _operation)) + { + // TODO remove from active +// state = State::Unused; + } + } +} -//bool UnusedStoreEliminator::knownUnrelated( -// UnusedStoreEliminator::Operation const& _op1, -// UnusedStoreEliminator::Operation const& _op2 -//) const -//{ -// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); +bool UnusedStoreEliminator::knownUnrelated( + UnusedStoreEliminator::Operation const& _op1, + UnusedStoreEliminator::Operation const& _op2 +) const +{ + KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); -// if (_op1.location != _op2.location) -// return true; -// if (_op1.location == Location::Storage) -// { -// if (_op1.start && _op2.start) -// { -// yulAssert( -// _op1.length && -// _op2.length && -// knowledge.valueIfKnownConstant(*_op1.length) == 1 && -// knowledge.valueIfKnownConstant(*_op2.length) == 1 -// ); -// return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); -// } -// } -// else -// { -// yulAssert(_op1.location == Location::Memory, ""); -// if ( -// (_op1.length && knowledge.knownToBeZero(*_op1.length)) || -// (_op2.length && knowledge.knownToBeZero(*_op2.length)) -// ) -// return true; + if (_op1.location != _op2.location) + return true; + if (_op1.location == Location::Storage) + { + if (_op1.start && _op2.start) + { + yulAssert( + _op1.length && + _op2.length && + knowledge.valueIfKnownConstant(*_op1.length) == 1 && + knowledge.valueIfKnownConstant(*_op2.length) == 1 + ); + return knowledge.knownToBeDifferent(*_op1.start, *_op2.start); + } + } + else + { + yulAssert(_op1.location == Location::Memory, ""); + if ( + (_op1.length && knowledge.knownToBeZero(*_op1.length)) || + (_op2.length && knowledge.knownToBeZero(*_op2.length)) + ) + return true; -// if (_op1.start && _op1.length && _op2.start) -// { -// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); -// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); -// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); -// if ( -// (length1 && start1 && start2) && -// *start1 + *length1 >= *start1 && // no overflow -// *start1 + *length1 <= *start2 -// ) -// return true; -// } -// if (_op2.start && _op2.length && _op1.start) -// { -// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); -// optional start2 = knowledge.valueIfKnownConstant(*_op2.start); -// optional start1 = knowledge.valueIfKnownConstant(*_op1.start); -// if ( -// (length2 && start2 && start1) && -// *start2 + *length2 >= *start2 && // no overflow -// *start2 + *length2 <= *start1 -// ) -// return true; -// } + if (_op1.start && _op1.length && _op2.start) + { + optional length1 = knowledge.valueIfKnownConstant(*_op1.length); + optional start1 = knowledge.valueIfKnownConstant(*_op1.start); + optional start2 = knowledge.valueIfKnownConstant(*_op2.start); + if ( + (length1 && start1 && start2) && + *start1 + *length1 >= *start1 && // no overflow + *start1 + *length1 <= *start2 + ) + return true; + } + if (_op2.start && _op2.length && _op1.start) + { + optional length2 = knowledge.valueIfKnownConstant(*_op2.length); + optional start2 = knowledge.valueIfKnownConstant(*_op2.start); + optional start1 = knowledge.valueIfKnownConstant(*_op1.start); + if ( + (length2 && start2 && start1) && + *start2 + *length2 >= *start2 && // no overflow + *start2 + *length2 <= *start1 + ) + return true; + } -// if (_op1.start && _op1.length && _op2.start && _op2.length) -// { -// optional length1 = knowledge.valueIfKnownConstant(*_op1.length); -// optional length2 = knowledge.valueIfKnownConstant(*_op2.length); -// if ( -// (length1 && *length1 <= 32) && -// (length2 && *length2 <= 32) && -// knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) -// ) -// return true; -// } -// } + if (_op1.start && _op1.length && _op2.start && _op2.length) + { + optional length1 = knowledge.valueIfKnownConstant(*_op1.length); + optional length2 = knowledge.valueIfKnownConstant(*_op2.length); + if ( + (length1 && *length1 <= 32) && + (length2 && *length2 <= 32) && + knowledge.knownToBeDifferentByAtLeast32(*_op1.start, *_op2.start) + ) + return true; + } + } -// return false; -//} + return false; +} -//bool UnusedStoreEliminator::knownCovered( -// UnusedStoreEliminator::Operation const& _covered, -// UnusedStoreEliminator::Operation const& _covering -//) const -//{ -// if (_covered.location != _covering.location) -// return false; -// if ( -// (_covered.start && _covered.start == _covering.start) && -// (_covered.length && _covered.length == _covering.length) -// ) -// return true; -// if (_covered.location == Location::Memory) -// { -// KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); +bool UnusedStoreEliminator::knownCovered( + UnusedStoreEliminator::Operation const& _covered, + UnusedStoreEliminator::Operation const& _covering +) const +{ + if (_covered.location != _covering.location) + return false; + if ( + (_covered.start && _covered.start == _covering.start) && + (_covered.length && _covered.length == _covering.length) + ) + return true; + if (_covered.location == Location::Memory) + { + KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); }); -// if (_covered.length && knowledge.knownToBeZero(*_covered.length)) -// return true; + if (_covered.length && knowledge.knownToBeZero(*_covered.length)) + return true; -// // Condition (i = cover_i_ng, e = cover_e_d): -// // i.start <= e.start && e.start + e.length <= i.start + i.length -// if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) -// return false; -// optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); -// optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); -// if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) -// if (coveredLength && coveringLength && *coveredLength <= *coveringLength) -// return true; -// optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); -// optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); -// if (coveredStart && coveringStart && coveredLength && coveringLength) -// if ( -// *coveringStart <= *coveredStart && -// *coveringStart + *coveringLength >= *coveringStart && // no overflow -// *coveredStart + *coveredLength >= *coveredStart && // no overflow -// *coveredStart + *coveredLength <= *coveringStart + *coveringLength -// ) -// return true; + // Condition (i = cover_i_ng, e = cover_e_d): + // i.start <= e.start && e.start + e.length <= i.start + i.length + if (!_covered.start || !_covering.start || !_covered.length || !_covering.length) + return false; + optional coveredLength = knowledge.valueIfKnownConstant(*_covered.length); + optional coveringLength = knowledge.valueIfKnownConstant(*_covering.length); + if (knowledge.knownToBeEqual(*_covered.start, *_covering.start)) + if (coveredLength && coveringLength && *coveredLength <= *coveringLength) + return true; + optional coveredStart = knowledge.valueIfKnownConstant(*_covered.start); + optional coveringStart = knowledge.valueIfKnownConstant(*_covering.start); + if (coveredStart && coveringStart && coveredLength && coveringLength) + if ( + *coveringStart <= *coveredStart && + *coveringStart + *coveringLength >= *coveringStart && // no overflow + *coveredStart + *coveredLength >= *coveredStart && // no overflow + *coveredStart + *coveredLength <= *coveringStart + *coveringLength + ) + return true; -// // TODO for this we probably need a non-overflow assumption as above. -// // Condition (i = cover_i_ng, e = cover_e_d): -// // i.start <= e.start && e.start + e.length <= i.start + i.length -// } -// return false; -//} + // TODO for this we probably need a non-overflow assumption as above. + // Condition (i = cover_i_ng, e = cover_e_d): + // i.start <= e.start && e.start + e.length <= i.start + i.length + } + return false; +} -//void UnusedStoreEliminator::changeUndecidedTo( -// State _newState, -// optional _onlyLocation) -//{ -// for (auto& [statement, state]: m_activeStores[YulString{}]) -// if ( -// state == State::Undecided && -// (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) -// ) -// state = _newState; -//} +void UnusedStoreEliminator::markActiveAsUsed( + optional _onlyLocation) +{ + // TODO it might make sense to use YulString{"m"} and YulString{"s"} for memory and storage. + // BUT: Could be both! + for (Statement const* statement: m_activeStores[YulString{}]) + if (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) + m_usedStores.insert(statement); + // TODO and remove from active +} -//optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const -//{ -// if (Identifier const* identifier = get_if(&_expression)) -// if (m_ssaValues.count(identifier->name)) -// return {identifier->name}; -// return nullopt; -//} +void UnusedStoreEliminator::changeUndecidedTo( + State _newState, + optional _onlyLocation) +{ + for (auto& [statement, state]: m_activeStores[YulString{}]) + if ( + state == State::Undecided && + (_onlyLocation == nullopt || *_onlyLocation == m_storeOperations.at(statement).location) + ) + state = _newState; +} -//void UnusedStoreEliminator::scheduleUnusedForDeletion() -//{ -// for (auto const& [statement, state]: m_activeStores[YulString{}]) -// if (state == State::Unused) -// m_pendingRemovals.insert(statement); -//} +optional UnusedStoreEliminator::identifierNameIfSSA(Expression const& _expression) const +{ + if (Identifier const* identifier = get_if(&_expression)) + if (m_ssaValues.count(identifier->name)) + return {identifier->name}; + return nullopt; +} + +void UnusedStoreEliminator::scheduleUnusedForDeletion() +{ + for (auto const& [statement, state]: m_activeStores[YulString{}]) + if (state == State::Unused) + m_pendingRemovals.insert(statement); +} diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h index 40ddd73b8..ce9c1fc16 100644 --- a/libyul/optimiser/UnusedStoreEliminator.h +++ b/libyul/optimiser/UnusedStoreEliminator.h @@ -70,57 +70,58 @@ public: bool// _ignoreMemory ): UnusedStoreBase(_dialect)//, -// m_ignoreMemory(_ignoreMemory), -// m_functionSideEffects(_functionSideEffects), -// m_controlFlowSideEffects(_controlFlowSideEffects), -// m_ssaValues(_ssaValues) + m_ignoreMemory(_ignoreMemory), + m_functionSideEffects(_functionSideEffects), + m_controlFlowSideEffects(_controlFlowSideEffects), + m_ssaValues(_ssaValues) {} -// using UnusedStoreBase::operator(); -// void operator()(FunctionCall const& _functionCall) override; -// void operator()(FunctionDefinition const&) override; -// void operator()(Leave const&) override; + using UnusedStoreBase::operator(); + void operator()(FunctionCall const& _functionCall) override; + void operator()(FunctionDefinition const&) override; + void operator()(Leave const&) override; -// using UnusedStoreBase::visit; -// void visit(Statement const& _statement) override; + using UnusedStoreBase::visit; + void visit(Statement const& _statement) override; -// using Location = evmasm::SemanticInformation::Location; -// using Effect = evmasm::SemanticInformation::Effect; -// struct Operation -// { -// Location location; -// Effect effect; -// /// Start of affected area. Unknown if not provided. -// std::optional start; -// /// Length of affected area, unknown if not provided. -// /// Unused for storage. -// std::optional length; -// }; + using Location = evmasm::SemanticInformation::Location; + using Effect = evmasm::SemanticInformation::Effect; + struct Operation + { + Location location; + Effect effect; + /// Start of affected area. Unknown if not provided. + std::optional start; + /// Length of affected area, unknown if not provided. + /// Unused for storage. + std::optional length; + }; private: void shortcutNestedLoop(ActiveStores const&) override { // We might only need to do this for newly introduced stores in the loop. -// changeUndecidedTo(State::Used); + changeUndecidedTo(State::Used); } -// void finalizeFunctionDefinition(FunctionDefinition const&) override; + void finalizeFunctionDefinition(FunctionDefinition const&) override; -// std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; -// void applyOperation(Operation const& _operation); -// bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; -// bool knownCovered(Operation const& _covered, Operation const& _covering) const; + std::vector operationsFromFunctionCall(FunctionCall const& _functionCall) const; + void applyOperation(Operation const& _operation); + bool knownUnrelated(Operation const& _op1, Operation const& _op2) const; + bool knownCovered(Operation const& _covered, Operation const& _covering) const; -// void changeUndecidedTo(State _newState, std::optional _onlyLocation = std::nullopt); -// void scheduleUnusedForDeletion(); + void markActiveAsUsed(std::optional _onlyLocation = std::nullopt); + void clearActive(std::optional _onlyLocation = std::nullopt); + void scheduleUnusedForDeletion(); -// std::optional identifierNameIfSSA(Expression const& _expression) const; + std::optional identifierNameIfSSA(Expression const& _expression) const; -// bool const m_ignoreMemory; -// std::map const& m_functionSideEffects; -// std::map m_controlFlowSideEffects; -// std::map const& m_ssaValues; + bool const m_ignoreMemory; + std::map const& m_functionSideEffects; + std::map m_controlFlowSideEffects; + std::map const& m_ssaValues; -// std::map m_storeOperations; + std::map m_storeOperations; }; } diff --git a/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul new file mode 100644 index 000000000..88b12264e --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedAssignEliminator/conditionally_assign_before_revert.yul @@ -0,0 +1,21 @@ +{ + let a := calldataload(0) + if calldataload(1) { + // this can be removed + a := 2 + revert(0, 0) + } + sstore(0, a) +} +// ---- +// step: unusedAssignEliminator +// +// { +// let a := calldataload(0) +// if calldataload(1) +// { +// a := 2 +// revert(0, 0) +// } +// sstore(0, a) +// }