Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove special yul strings from UnusedStoreEliminator #15337

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
+50 −41
Draft

Remove special yul strings from UnusedStoreEliminator #15337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5f5f5a
Remove special "m" and "s" strings from UnusedStoreEliminator.
ekpyron Aug 15, 2024
33b818c
Replace special names for literals in UnusedStoreEliminator.
ekpyron Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libyul/optimiser/UnusedAssignEliminator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ struct Dialect;
*
* Prerequisite: Disambiguator, ForLoopInitRewriter.
*/
class UnusedAssignEliminator: public UnusedStoreBase
class UnusedAssignEliminator: public UnusedStoreBase<YulName>
{
public:
static constexpr char const* name{"UnusedAssignEliminator"};
Expand Down
27 changes: 19 additions & 8 deletions libyul/optimiser/UnusedStoreBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
using namespace solidity;
using namespace solidity::yul;

void UnusedStoreBase::operator()(If const& _if)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::operator()(If const& _if)
{
visit(*_if.condition);

Expand All @@ -42,7 +43,8 @@ void UnusedStoreBase::operator()(If const& _if)
merge(m_activeStores, std::move(skipBranch));
}

void UnusedStoreBase::operator()(Switch const& _switch)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::operator()(Switch const& _switch)
{
visit(*_switch.expression);

Expand All @@ -68,7 +70,8 @@ void UnusedStoreBase::operator()(Switch const& _switch)
merge(m_activeStores, std::move(branch));
}

void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::operator()(FunctionDefinition const& _functionDefinition)
{
ScopedSaveAndRestore allStores(m_allStores, {});
ScopedSaveAndRestore usedStores(m_usedStores, {});
Expand All @@ -82,7 +85,8 @@ void UnusedStoreBase::operator()(FunctionDefinition const& _functionDefinition)
m_storesToRemove += m_allStores - m_usedStores;
}

void UnusedStoreBase::operator()(ForLoop const& _forLoop)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::operator()(ForLoop const& _forLoop)
{
ScopedSaveAndRestore outerForLoopInfo(m_forLoopInfo, {});
ScopedSaveAndRestore forLoopNestingDepth(m_forLoopNestingDepth, m_forLoopNestingDepth + 1);
Expand Down Expand Up @@ -130,19 +134,22 @@ void UnusedStoreBase::operator()(ForLoop const& _forLoop)
m_forLoopInfo.pendingBreakStmts.clear();
}

void UnusedStoreBase::operator()(Break const&)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::operator()(Break const&)
{
m_forLoopInfo.pendingBreakStmts.emplace_back(std::move(m_activeStores));
m_activeStores.clear();
}

void UnusedStoreBase::operator()(Continue const&)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::operator()(Continue const&)
{
m_forLoopInfo.pendingContinueStmts.emplace_back(std::move(m_activeStores));
m_activeStores.clear();
}

void UnusedStoreBase::merge(ActiveStores& _target, ActiveStores&& _other)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::merge(ActiveStores& _target, ActiveStores&& _other)
{
util::joinMap(_target, std::move(_other), [](
std::set<Statement const*>& _storesHere,
Expand All @@ -153,9 +160,13 @@ void UnusedStoreBase::merge(ActiveStores& _target, ActiveStores&& _other)
});
}

void UnusedStoreBase::merge(ActiveStores& _target, std::vector<ActiveStores>&& _source)
template<typename ActiveStoresKeyType>
void UnusedStoreBase<ActiveStoresKeyType>::merge(ActiveStores& _target, std::vector<ActiveStores>&& _source)
{
for (ActiveStores& ts: _source)
merge(_target, std::move(ts));
_source.clear();
}

template class solidity::yul::UnusedStoreBase<UnusedStoreEliminatorKey>;
template class solidity::yul::UnusedStoreBase<YulName>;
6 changes: 5 additions & 1 deletion libyul/optimiser/UnusedStoreBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ struct Dialect;
*
* Prerequisite: Disambiguator, ForLoopInitRewriter.
*/
template<typename ActiveStoreKeyType>
class UnusedStoreBase: public ASTWalker
{
public:
Expand All @@ -60,7 +61,7 @@ class UnusedStoreBase: public ASTWalker
void operator()(Continue const&) override;

protected:
using ActiveStores = std::map<YulName, std::set<Statement const*>>;
using ActiveStores = std::map<ActiveStoreKeyType, std::set<Statement const*>>;

/// This function is called for a loop that is nested too deep to avoid
/// horrible runtime and should just resolve the situation in a pragmatic
Expand Down Expand Up @@ -98,4 +99,7 @@ class UnusedStoreBase: public ASTWalker
size_t m_forLoopNestingDepth = 0;
};

enum class UnusedStoreEliminatorKey { Memory, Storage };
extern template class UnusedStoreBase<YulString>;
extern template class UnusedStoreBase<UnusedStoreEliminatorKey>;
}
38 changes: 13 additions & 25 deletions libyul/optimiser/UnusedStoreEliminator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@
using namespace solidity;
using namespace solidity::yul;

/// Variable names for special constants that can never appear in actual Yul code.
static std::string const zero{"@ 0"};
static std::string const one{"@ 1"};
static std::string const thirtyTwo{"@ 32"};


void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast)
{
std::map<YulName, SideEffects> functionSideEffects = SideEffectsPropagator::sideEffects(
Expand All @@ -61,12 +55,6 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast)
std::map<YulName, AssignedValue> values;
for (auto const& [name, expression]: ssaValues.values())
values[name] = AssignedValue{expression, {}};
Expression const zeroLiteral{Literal{{}, LiteralKind::Number, LiteralValue(u256{0}), {}}};
Expression const oneLiteral{Literal{{}, LiteralKind::Number, LiteralValue(u256{1}), {}}};
Expression const thirtyTwoLiteral{Literal{{}, LiteralKind::Number, LiteralValue(u256{32}), {}}};
values[YulName{zero}] = AssignedValue{&zeroLiteral, {}};
values[YulName{one}] = AssignedValue{&oneLiteral, {}};
values[YulName{thirtyTwo}] = AssignedValue{&thirtyTwoLiteral, {}};

bool const ignoreMemory = MSizeFinder::containsMSize(_context.dialect, _ast);
UnusedStoreEliminator rse{
Expand Down Expand Up @@ -263,8 +251,8 @@ std::vector<UnusedStoreEliminator::Operation> UnusedStoreEliminator::operationsF
if (_op.lengthConstant)
switch (*_op.lengthConstant)
{
case 1: ourOp.length = YulName(one); break;
case 32: ourOp.length = YulName(thirtyTwo); break;
case 1: ourOp.length = u256(1); break;
case 32: ourOp.length = u256(32); break;
default: yulAssert(false);
}
return ourOp;
Expand Down Expand Up @@ -312,8 +300,8 @@ bool UnusedStoreEliminator::knownUnrelated(
yulAssert(
_op1.length &&
_op2.length &&
m_knowledgeBase.valueIfKnownConstant(*_op1.length) == 1 &&
m_knowledgeBase.valueIfKnownConstant(*_op2.length) == 1
lengthValue(*_op1.length) == 1 &&
lengthValue(*_op2.length) == 1
);
return m_knowledgeBase.knownToBeDifferent(*_op1.start, *_op2.start);
}
Expand All @@ -322,14 +310,14 @@ bool UnusedStoreEliminator::knownUnrelated(
{
yulAssert(_op1.location == Location::Memory, "");
if (
(_op1.length && m_knowledgeBase.knownToBeZero(*_op1.length)) ||
(_op2.length && m_knowledgeBase.knownToBeZero(*_op2.length))
(_op1.length && lengthValue(*_op1.length) == 0) ||
(_op2.length && lengthValue(*_op2.length) == 0)
)
return true;

if (_op1.start && _op1.length && _op2.start)
{
std::optional<u256> length1 = m_knowledgeBase.valueIfKnownConstant(*_op1.length);
std::optional<u256> length1 = lengthValue(*_op1.length);
std::optional<u256> start1 = m_knowledgeBase.valueIfKnownConstant(*_op1.start);
std::optional<u256> start2 = m_knowledgeBase.valueIfKnownConstant(*_op2.start);
if (
Expand All @@ -341,7 +329,7 @@ bool UnusedStoreEliminator::knownUnrelated(
}
if (_op2.start && _op2.length && _op1.start)
{
std::optional<u256> length2 = m_knowledgeBase.valueIfKnownConstant(*_op2.length);
std::optional<u256> length2 = lengthValue(*_op2.length);
std::optional<u256> start2 = m_knowledgeBase.valueIfKnownConstant(*_op2.start);
std::optional<u256> start1 = m_knowledgeBase.valueIfKnownConstant(*_op1.start);
if (
Expand All @@ -354,8 +342,8 @@ bool UnusedStoreEliminator::knownUnrelated(

if (_op1.start && _op1.length && _op2.start && _op2.length)
{
std::optional<u256> length1 = m_knowledgeBase.valueIfKnownConstant(*_op1.length);
std::optional<u256> length2 = m_knowledgeBase.valueIfKnownConstant(*_op2.length);
std::optional<u256> length1 = lengthValue(*_op1.length);
std::optional<u256> length2 = lengthValue(*_op2.length);
if (
(length1 && *length1 <= 32) &&
(length2 && *length2 <= 32) &&
Expand All @@ -382,15 +370,15 @@ bool UnusedStoreEliminator::knownCovered(
return true;
if (_covered.location == Location::Memory)
{
if (_covered.length && m_knowledgeBase.knownToBeZero(*_covered.length))
if (_covered.length && lengthValue(*_covered.length) == 0)
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;
std::optional<u256> coveredLength = m_knowledgeBase.valueIfKnownConstant(*_covered.length);
std::optional<u256> coveringLength = m_knowledgeBase.valueIfKnownConstant(*_covering.length);
std::optional<u256> coveredLength = lengthValue(*_covered.length);
std::optional<u256> coveringLength = lengthValue(*_covering.length);
if (*_covered.start == *_covering.start)
if (coveredLength && coveringLength && *coveredLength <= *coveringLength)
return true;
Expand Down
18 changes: 12 additions & 6 deletions libyul/optimiser/UnusedStoreEliminator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ struct AssignedValue;
* to sstore, as we don't know whether the memory location will be read once we leave the function's scope,
* so the statement will be removed only if all code code paths lead to a memory overwrite.
*
* The m_store member of UnusedStoreBase uses the key "m" for memory and "s" for storage stores.
*
* Best run in SSA form.
*
* Prerequisite: Disambiguator, ForLoopInitRewriter.
*/
class UnusedStoreEliminator: public UnusedStoreBase
class UnusedStoreEliminator: public UnusedStoreBase<UnusedStoreEliminatorKey>
{
public:
static constexpr char const* name{"UnusedStoreEliminator"};
Expand All @@ -80,6 +78,7 @@ class UnusedStoreEliminator: public UnusedStoreBase

using Location = evmasm::SemanticInformation::Location;
using Effect = evmasm::SemanticInformation::Effect;
using OperationLength = std::variant<YulName, u256>;
struct Operation
{
Location location;
Expand All @@ -88,12 +87,19 @@ class UnusedStoreEliminator: public UnusedStoreBase
std::optional<YulName> start;
/// Length of affected area, unknown if not provided.
/// Unused for storage.
std::optional<YulName> length;
std::optional<OperationLength> length;
};

private:
std::set<Statement const*>& activeMemoryStores() { return m_activeStores["m"_yulname]; }
std::set<Statement const*>& activeStorageStores() { return m_activeStores["s"_yulname]; }
std::set<Statement const*>& activeMemoryStores() { return m_activeStores[UnusedStoreEliminatorKey::Memory]; }
std::set<Statement const*>& activeStorageStores() { return m_activeStores[UnusedStoreEliminatorKey::Storage]; }
std::optional<u256> lengthValue(OperationLength const& _length) const
{
if (YulName const* length = std::get_if<YulName>(&_length))
return m_knowledgeBase.valueIfKnownConstant(*length);
else
return std::get<u256>(_length);
}

void shortcutNestedLoop(ActiveStores const&) override
{
Expand Down