Skip to content

Commit

Permalink
Revert change in CommonSubexpressionEliminator
Browse files Browse the repository at this point in the history
  • Loading branch information
r0qs committed Dec 1, 2022
1 parent a6a03ba commit 553aae5
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 29 deletions.
4 changes: 2 additions & 2 deletions libevmasm/Assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,9 @@ map<u256, u256> const& Assembly::optimiseInternal(
while (iter != m_items.end())
{
KnownState emptyState;
CommonSubexpressionEliminator eliminator{emptyState, _settings.evmVersion};
CommonSubexpressionEliminator eliminator{emptyState};
auto orig = iter;
iter = eliminator.feedItems(iter, m_items.end(), usesMSize);
iter = eliminator.feedItems(_settings.evmVersion, iter, m_items.end(), usesMSize);
bool shouldReplace = false;
AssemblyItems optimisedChunk;
try
Expand Down
18 changes: 8 additions & 10 deletions libevmasm/CommonSubexpressionEliminator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ vector<AssemblyItem> CommonSubexpressionEliminator::getOptimizedItems()
m_initialState.sequenceNumber(),
m_initialState.stackHeight(),
initialStackContents,
targetStackContents,
m_evmVersion
targetStackContents
);
if (m_breakingItem)
items.push_back(*m_breakingItem);
Expand Down Expand Up @@ -138,8 +137,7 @@ AssemblyItems CSECodeGenerator::generateCode(
unsigned _initialSequenceNumber,
int _initialStackHeight,
map<int, Id> const& _initialStack,
map<int, Id> const& _targetStackContents,
EVMVersion _evmVersion
map<int, Id> const& _targetStackContents
)
{
m_stackHeight = _initialStackHeight;
Expand Down Expand Up @@ -173,14 +171,14 @@ AssemblyItems CSECodeGenerator::generateCode(
// Perform all operations on storage and memory in order, if they are needed.
for (auto const& seqAndId: sequencedExpressions)
if (!m_classPositions.count(seqAndId.second))
generateClassElement(seqAndId.second, _evmVersion, true);
generateClassElement(seqAndId.second, true);

// generate the target stack elements
for (auto const& targetItem: m_targetStack)
{
if (m_stack.count(targetItem.first) && m_stack.at(targetItem.first) == targetItem.second)
continue; // already there
generateClassElement(targetItem.second, _evmVersion);
generateClassElement(targetItem.second);
assertThrow(!m_classPositions[targetItem.second].empty(), OptimizerException, "");
if (m_classPositions[targetItem.second].count(targetItem.first))
continue;
Expand Down Expand Up @@ -301,7 +299,7 @@ void CSECodeGenerator::addDependencies(Id _c)
}
}

void CSECodeGenerator::generateClassElement(Id _c, langutil::EVMVersion _evmVersion, bool _allowSequenced)
void CSECodeGenerator::generateClassElement(Id _c, bool _allowSequenced)
{
for (auto const& it: m_classPositions)
for (int p: it.second)
Expand Down Expand Up @@ -335,7 +333,7 @@ void CSECodeGenerator::generateClassElement(Id _c, langutil::EVMVersion _evmVers
);
vector<Id> const& arguments = expr.arguments;
for (Id arg: arguments | ranges::views::reverse)
generateClassElement(arg, _evmVersion);
generateClassElement(arg);

SourceLocation const& itemLocation = expr.item->location();
// The arguments are somewhere on the stack now, so it remains to move them at the correct place.
Expand Down Expand Up @@ -408,15 +406,15 @@ void CSECodeGenerator::generateClassElement(Id _c, langutil::EVMVersion _evmVers
m_stack.erase(m_stackHeight - static_cast<int>(i));
}
appendItem(*expr.item);
if (expr.item->type() != Operation || instructionInfo(expr.item->instruction(), _evmVersion).ret == 1)
if (expr.item->type() != Operation || instructionInfo(expr.item->instruction(), EVMVersion()).ret == 1)
{
m_stack[m_stackHeight] = _c;
m_classPositions[_c].insert(m_stackHeight);
}
else
{
assertThrow(
instructionInfo(expr.item->instruction(), _evmVersion).ret == 0,
instructionInfo(expr.item->instruction(), EVMVersion()).ret == 0,
OptimizerException,
"Invalid number of return values."
);
Expand Down
15 changes: 6 additions & 9 deletions libevmasm/CommonSubexpressionEliminator.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include <libevmasm/ExpressionClasses.h>
#include <libevmasm/SemanticInformation.h>
#include <libevmasm/KnownState.h>
#include <liblangutil/EVMVersion.h>

namespace langutil
{
Expand Down Expand Up @@ -67,13 +66,13 @@ class CommonSubexpressionEliminator
using Id = ExpressionClasses::Id;
using StoreOperation = KnownState::StoreOperation;

explicit CommonSubexpressionEliminator(KnownState const& _state, langutil::EVMVersion _evmVersion): m_initialState(_state), m_state(_state), m_evmVersion(_evmVersion) {}
explicit CommonSubexpressionEliminator(KnownState const& _state): m_initialState(_state), m_state(_state) {}

/// Feeds AssemblyItems into the eliminator and @returns the iterator pointing at the first
/// item that must be fed into a new instance of the eliminator.
/// @param _msizeImportant if false, do not consider modification of MSIZE a side-effect
template <class AssemblyItemIterator>
AssemblyItemIterator feedItems(AssemblyItemIterator _iterator, AssemblyItemIterator _end, bool _msizeImportant);
AssemblyItemIterator feedItems(langutil::EVMVersion _evmVersion, AssemblyItemIterator _iterator, AssemblyItemIterator _end, bool _msizeImportant);

/// @returns the resulting items after optimization.
AssemblyItems getOptimizedItems();
Expand All @@ -94,8 +93,6 @@ class CommonSubexpressionEliminator
/// The item that breaks the basic block, can be nullptr.
/// It is usually appended to the block but can be optimized in some cases.
AssemblyItem const* m_breakingItem = nullptr;

langutil::EVMVersion m_evmVersion;
};

/**
Expand Down Expand Up @@ -123,8 +120,7 @@ class CSECodeGenerator
unsigned _initialSequenceNumber,
int _initialStackHeight,
std::map<int, Id> const& _initialStack,
std::map<int, Id> const& _targetStackContents,
langutil::EVMVersion _evmVersion
std::map<int, Id> const& _targetStackContents
);

private:
Expand All @@ -133,7 +129,7 @@ class CSECodeGenerator

/// Produce code that generates the given element if it is not yet present.
/// @param _allowSequenced indicates that sequence-constrained operations are allowed
void generateClassElement(Id _c, langutil::EVMVersion _evmVersion, bool _allowSequenced = false);
void generateClassElement(Id _c, bool _allowSequenced = false);
/// @returns the position of the representative of the given id on the stack.
/// @note throws an exception if it is not on the stack.
int classElementPosition(Id _id) const;
Expand Down Expand Up @@ -177,6 +173,7 @@ class CSECodeGenerator

template <class AssemblyItemIterator>
AssemblyItemIterator CommonSubexpressionEliminator::feedItems(
langutil::EVMVersion _evmVersion,
AssemblyItemIterator _iterator,
AssemblyItemIterator _end,
bool _msizeImportant
Expand All @@ -187,7 +184,7 @@ AssemblyItemIterator CommonSubexpressionEliminator::feedItems(
unsigned chunkSize = 0;
for (
;
_iterator != _end && !SemanticInformation::breaksCSEAnalysisBlock(*_iterator, _msizeImportant, m_evmVersion) && chunkSize < maxChunkSize;
_iterator != _end && !SemanticInformation::breaksCSEAnalysisBlock(*_iterator, _msizeImportant, _evmVersion) && chunkSize < maxChunkSize;
++_iterator, ++chunkSize
)
feedItem(*_iterator);
Expand Down
16 changes: 8 additions & 8 deletions test/libevmasm/Optimiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ namespace
bool usesMsize = ranges::any_of(_input, [](AssemblyItem const& _i) {
return _i == AssemblyItem{Instruction::MSIZE} || _i.type() == VerbatimBytecode;
});
evmasm::CommonSubexpressionEliminator cse(_state, solidity::test::CommonOptions::get().evmVersion());
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), usesMsize) == input.end());
evmasm::CommonSubexpressionEliminator cse(_state);
BOOST_REQUIRE(cse.feedItems(solidity::test::CommonOptions::get().evmVersion(), input.begin(), input.end(), usesMsize) == input.end());
AssemblyItems output = cse.getOptimizedItems();

for (AssemblyItem const& item: output)
Expand Down Expand Up @@ -107,9 +107,9 @@ namespace
while (iter != _input.end())
{
KnownState emptyState;
CommonSubexpressionEliminator eliminator{emptyState, solidity::test::CommonOptions::get().evmVersion()};
CommonSubexpressionEliminator eliminator{emptyState};
auto orig = iter;
iter = eliminator.feedItems(iter, _input.end(), usesMSize);
iter = eliminator.feedItems(solidity::test::CommonOptions::get().evmVersion(), iter, _input.end(), usesMSize);
bool shouldReplace = false;
AssemblyItems optimisedChunk;
optimisedChunk = eliminator.getOptimizedItems();
Expand Down Expand Up @@ -190,21 +190,21 @@ BOOST_AUTO_TEST_CASE(cse_assign_immutable_breaks)
Instruction::ORIGIN
});

evmasm::CommonSubexpressionEliminator cse{evmasm::KnownState(), solidity::test::CommonOptions::get().evmVersion()};
evmasm::CommonSubexpressionEliminator cse{evmasm::KnownState()};
// Make sure CSE breaks after AssignImmutable.
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), false) == input.begin() + 2);
BOOST_REQUIRE(cse.feedItems(solidity::test::CommonOptions::get().evmVersion(), input.begin(), input.end(), false) == input.begin() + 2);
}

BOOST_AUTO_TEST_CASE(cse_intermediate_swap)
{
evmasm::KnownState state;
evmasm::CommonSubexpressionEliminator cse(state, solidity::test::CommonOptions::get().evmVersion());
evmasm::CommonSubexpressionEliminator cse(state);
AssemblyItems input{
Instruction::SWAP1, Instruction::POP, Instruction::ADD, u256(0), Instruction::SWAP1,
Instruction::SLOAD, Instruction::SWAP1, u256(100), Instruction::EXP, Instruction::SWAP1,
Instruction::DIV, u256(0xff), Instruction::AND
};
BOOST_REQUIRE(cse.feedItems(input.begin(), input.end(), false) == input.end());
BOOST_REQUIRE(cse.feedItems(solidity::test::CommonOptions::get().evmVersion(), input.begin(), input.end(), false) == input.end());
AssemblyItems output = cse.getOptimizedItems();
BOOST_CHECK(!output.empty());
}
Expand Down

0 comments on commit 553aae5

Please sign in to comment.