From 965f005a989e0b7fb5a46f364bfb96dad2e18aae Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 24 Mar 2015 14:53:15 +0100 Subject: [PATCH 01/13] Storage access optimisation. --- libevmcore/CommonSubexpressionEliminator.cpp | 205 ++++++++++++++----- libevmcore/CommonSubexpressionEliminator.h | 67 +++++- libevmcore/ExpressionClasses.cpp | 70 +++++-- libevmcore/ExpressionClasses.h | 10 +- test/SolidityOptimizer.cpp | 180 ++++++++++++++++ 5 files changed, 453 insertions(+), 79 deletions(-) diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 7fed03b4e..43d01fc85 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -45,19 +45,24 @@ vector CommonSubexpressionEliminator::getOptimizedItems() // Debug info: //stream(cout, initialStackContents, targetStackContents); - return CSECodeGenerator(m_expressionClasses).generateCode(initialStackContents, targetStackContents); + return CSECodeGenerator(m_expressionClasses, m_storageWrites).generateCode( + initialStackContents, + targetStackContents + ); } ostream& CommonSubexpressionEliminator::stream( ostream& _out, - map _currentStack, + map _initialStack, map _targetStack ) const { auto streamExpressionClass = [this](ostream& _out, ExpressionClasses::Id _id) { auto const& expr = m_expressionClasses.representative(_id); - _out << " " << _id << ": " << *expr.item; + _out << " " << dec << _id << ": " << *expr.item; + if (expr.sequenceNumber) + _out << "@" << dec << expr.sequenceNumber; _out << "("; for (ExpressionClasses::Id arg: expr.arguments) _out << dec << arg << ","; @@ -66,18 +71,12 @@ ostream& CommonSubexpressionEliminator::stream( _out << "Optimizer analysis:" << endl; _out << "Final stack height: " << dec << m_stackHeight << endl; - _out << "Stack elements: " << endl; - for (auto const& it: m_stackElements) - { - _out << " " << dec << it.first << " = "; - streamExpressionClass(_out, it.second); - } _out << "Equivalence classes: " << endl; for (ExpressionClasses::Id eqClass = 0; eqClass < m_expressionClasses.size(); ++eqClass) streamExpressionClass(_out, eqClass); - _out << "Current stack: " << endl; - for (auto const& it: _currentStack) + _out << "Initial stack: " << endl; + for (auto const& it: _initialStack) { _out << " " << dec << it.first << ": "; streamExpressionClass(_out, it.second); @@ -96,9 +95,8 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) { if (_item.type() != Operation) { - if (_item.deposit() != 1) - BOOST_THROW_EXCEPTION(InvalidDeposit()); - setStackElement(++m_stackHeight, m_expressionClasses.find(_item, {})); + assertThrow(_item.deposit() == 1, InvalidDeposit, ""); + setStackElement(++m_stackHeight, m_expressionClasses.find(_item, {}, false)); } else { @@ -119,7 +117,12 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) vector arguments(info.args); for (int i = 0; i < info.args; ++i) arguments[i] = stackElement(m_stackHeight - i); - setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments)); + if (_item.instruction() == Instruction::SSTORE) + storeInStorage(arguments[0], arguments[1]); + else if (_item.instruction() == Instruction::SLOAD) + setStackElement(m_stackHeight + _item.deposit(), loadFromStorage(arguments[0])); + else + setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments, false)); } m_stackHeight += _item.deposit(); } @@ -132,8 +135,7 @@ void CommonSubexpressionEliminator::setStackElement(int _stackHeight, Expression void CommonSubexpressionEliminator::swapStackElements(int _stackHeightA, int _stackHeightB) { - if (_stackHeightA == _stackHeightB) - BOOST_THROW_EXCEPTION(OptimizerException() << errinfo_comment("Swap on same stack elements.")); + assertThrow(_stackHeightA != _stackHeightB, OptimizerException, "Swap on same stack elements."); // ensure they are created stackElement(_stackHeightA); stackElement(_stackHeightB); @@ -157,6 +159,33 @@ ExpressionClasses::Id CommonSubexpressionEliminator::initialStackElement(int _st return m_expressionClasses.find(AssemblyItem(dupInstruction(1 - _stackHeight))); } +void CommonSubexpressionEliminator::storeInStorage(ExpressionClasses::Id _slot, ExpressionClasses::Id _value) +{ + if (m_storageContent.count(_slot) && m_storageContent[_slot] == _value) + // do not execute the storage if we know that the value is already there + return; + m_sequenceNumber ++; + decltype(m_storageContent) storageContents; + // copy over values at points where we know that they are different from _slot + for (auto const& storageItem: m_storageContent) + if (m_expressionClasses.knownToBeDifferent(storageItem.first, _slot)) + storageContents.insert(storageItem); + m_storageContent = move(storageContents); + ExpressionClasses::Id id = m_expressionClasses.find(Instruction::SSTORE, {_slot, _value}, true, m_sequenceNumber); + m_storageWrites.insert(StorageWriteOperation(_slot, m_sequenceNumber, id)); + m_storageContent[_slot] = _value; + // increment a second time so that we get unique sequence numbers for writes + m_sequenceNumber ++; +} + +ExpressionClasses::Id CommonSubexpressionEliminator::loadFromStorage(ExpressionClasses::Id _slot) +{ + if (m_storageContent.count(_slot)) + return m_storageContent.at(_slot); + else + return m_storageContent[_slot] = m_expressionClasses.find(Instruction::SLOAD, {_slot}, true, m_sequenceNumber); +} + bool SemanticInformation::breaksBasicBlock(AssemblyItem const& _item) { switch (_item.type()) @@ -180,6 +209,8 @@ bool SemanticInformation::breaksBasicBlock(AssemblyItem const& _item) if (_item.instruction() == Instruction::GAS || _item.instruction() == Instruction::PC) return true; // GAS and PC assume a specific order of opcodes InstructionInfo info = instructionInfo(_item.instruction()); + if (_item.instruction() == Instruction::SSTORE) + return false; // the second requirement will be lifted once it is implemented return info.sideEffects || info.args > 2; } @@ -230,26 +261,49 @@ AssemblyItems CSECodeGenerator::generateCode( // @todo: provide information about the positions of copies of class elements - // generate the dependency graph + // generate the dependency graph starting from final storage writes and target stack contents + for (auto it = m_storageWrites.begin(); it != m_storageWrites.end();) + { + auto next = it; + ++next; + if (next == m_storageWrites.end() || next->slot != it->slot) + // last write to that storage slot + addDependencies(it->expression); + it = next; + } for (auto const& targetItem: _targetStackContents) { m_finalClasses.insert(targetItem.second); addDependencies(targetItem.second); } - // generate the actual elements + // Perform all operations on storage in order, if they are needed. + //@todo use better data structures to optimize these loops + unsigned maxSequenceNumber = 1; + for (StorageWriteOperation const& op: m_storageWrites) + maxSequenceNumber = max(maxSequenceNumber, op.sequenceNumber + 1); + for (unsigned sequenceNumber = 1; sequenceNumber <= maxSequenceNumber; ++sequenceNumber) + for (auto const& depPair: m_neededBy) + for (ExpressionClasses::Id const& id: {depPair.first, depPair.second}) + if ( + m_expressionClasses.representative(id).sequenceNumber == sequenceNumber && + !m_classPositions.count(id) + ) + generateClassElement(id, true); + + // generate the target stack elements for (auto const& targetItem: _targetStackContents) { - removeStackTopIfPossible(); int position = generateClassElement(targetItem.second); + assertThrow(position != c_invalidPosition, OptimizerException, ""); if (position == targetItem.first) continue; if (position < targetItem.first) // it is already at its target, we need another copy appendDup(position); else - appendSwapOrRemove(position); - appendSwapOrRemove(targetItem.first); + appendOrRemoveSwap(position); + appendOrRemoveSwap(targetItem.first); } // remove surplus elements @@ -270,23 +324,48 @@ AssemblyItems CSECodeGenerator::generateCode( // neither initial no target stack, no change in height finalHeight = 0; assertThrow(finalHeight == m_stackHeight, OptimizerException, "Incorrect final stack height."); - return m_generatedItems; } void CSECodeGenerator::addDependencies(ExpressionClasses::Id _c) { if (m_neededBy.count(_c)) - return; - for (ExpressionClasses::Id argument: m_expressionClasses.representative(_c).arguments) + return; // we already computed the dependencies for _c + ExpressionClasses::Expression const& expr = m_expressionClasses.representative(_c); + for (ExpressionClasses::Id argument: expr.arguments) { addDependencies(argument); m_neededBy.insert(make_pair(argument, _c)); } + if (expr.item->type() == Operation && expr.item->instruction() == Instruction::SLOAD) + { + // this loads an unknown value from storage and thus, in addition to its arguments, depends + // on all SSTORE operations to addresses where we do not know that they are different that + // occur before this SLOAD + ExpressionClasses::Id slotToLoadFrom = expr.arguments.at(0); + for (auto it = m_storageWrites.begin(); it != m_storageWrites.end();) + { + auto next = it; + ++next; + // note that SSTORE and SLOAD never have the same sequence number + if (it->sequenceNumber < expr.sequenceNumber && + !m_expressionClasses.knownToBeDifferent(it->slot, slotToLoadFrom) && + (next == m_storageWrites.end() || next->sequenceNumber > expr.sequenceNumber) + ) + { + addDependencies(it->expression); + m_neededBy.insert(make_pair(it->expression, _c)); + } + it = next; + } + } } -int CSECodeGenerator::generateClassElement(ExpressionClasses::Id _c) +int CSECodeGenerator::generateClassElement(ExpressionClasses::Id _c, bool _allowSequenced) { + // do some cleanup + removeStackTopIfPossible(); + if (m_classPositions.count(_c)) { assertThrow( @@ -296,7 +375,13 @@ int CSECodeGenerator::generateClassElement(ExpressionClasses::Id _c) ); return m_classPositions[_c]; } - ExpressionClasses::Ids const& arguments = m_expressionClasses.representative(_c).arguments; + ExpressionClasses::Expression const& expr = m_expressionClasses.representative(_c); + assertThrow( + _allowSequenced || expr.sequenceNumber == 0, + OptimizerException, + "Sequence constrained operation requested out of sequence." + ); + ExpressionClasses::Ids const& arguments = expr.arguments; for (ExpressionClasses::Id arg: boost::adaptors::reverse(arguments)) generateClassElement(arg); @@ -307,42 +392,42 @@ int CSECodeGenerator::generateClassElement(ExpressionClasses::Id _c) if (arguments.size() == 1) { if (canBeRemoved(arguments[0], _c)) - appendSwapOrRemove(generateClassElement(arguments[0])); + appendOrRemoveSwap(classElementPosition(arguments[0])); else - appendDup(generateClassElement(arguments[0])); + appendDup(classElementPosition(arguments[0])); } else if (arguments.size() == 2) { if (canBeRemoved(arguments[1], _c)) { - appendSwapOrRemove(generateClassElement(arguments[1])); + appendOrRemoveSwap(classElementPosition(arguments[1])); if (arguments[0] == arguments[1]) appendDup(m_stackHeight); else if (canBeRemoved(arguments[0], _c)) { - appendSwapOrRemove(m_stackHeight - 1); - appendSwapOrRemove(generateClassElement(arguments[0])); + appendOrRemoveSwap(m_stackHeight - 1); + appendOrRemoveSwap(classElementPosition(arguments[0])); } else - appendDup(generateClassElement(arguments[0])); + appendDup(classElementPosition(arguments[0])); } else { if (arguments[0] == arguments[1]) { - appendDup(generateClassElement(arguments[0])); + appendDup(classElementPosition(arguments[0])); appendDup(m_stackHeight); } else if (canBeRemoved(arguments[0], _c)) { - appendSwapOrRemove(generateClassElement(arguments[0])); - appendDup(generateClassElement(arguments[1])); - appendSwapOrRemove(m_stackHeight - 1); + appendOrRemoveSwap(classElementPosition(arguments[0])); + appendDup(classElementPosition(arguments[1])); + appendOrRemoveSwap(m_stackHeight - 1); } else { - appendDup(generateClassElement(arguments[1])); - appendDup(generateClassElement(arguments[0])); + appendDup(classElementPosition(arguments[1])); + appendDup(classElementPosition(arguments[0])); } } } @@ -355,20 +440,41 @@ int CSECodeGenerator::generateClassElement(ExpressionClasses::Id _c) for (size_t i = 0; i < arguments.size(); ++i) assertThrow(m_stack[m_stackHeight - i] == arguments[i], OptimizerException, "Expected arguments not present." ); - AssemblyItem const& item = *m_expressionClasses.representative(_c).item; - while (SemanticInformation::isCommutativeOperation(item) && + while (SemanticInformation::isCommutativeOperation(*expr.item) && !m_generatedItems.empty() && m_generatedItems.back() == AssemblyItem(Instruction::SWAP1)) // this will not append a swap but remove the one that is already there - appendSwapOrRemove(m_stackHeight - 1); + appendOrRemoveSwap(m_stackHeight - 1); for (auto arg: arguments) if (canBeRemoved(arg, _c)) m_classPositions[arg] = c_invalidPosition; for (size_t i = 0; i < arguments.size(); ++i) m_stack.erase(m_stackHeight - i); - appendItem(*m_expressionClasses.representative(_c).item); - m_stack[m_stackHeight] = _c; - return m_classPositions[_c] = m_stackHeight; + appendItem(*expr.item); + if (expr.item->type() != Operation || instructionInfo(expr.item->instruction()).ret == 1) + { + m_stack[m_stackHeight] = _c; + return m_classPositions[_c] = m_stackHeight; + } + else + { + assertThrow( + instructionInfo(expr.item->instruction()).ret == 0, + OptimizerException, + "Invalid number of return values." + ); + return m_classPositions[_c] = c_invalidPosition; + } +} + +int CSECodeGenerator::classElementPosition(ExpressionClasses::Id _id) const +{ + assertThrow( + m_classPositions.count(_id) && m_classPositions.at(_id) != c_invalidPosition, + OptimizerException, + "Element requested but is not present." + ); + return m_classPositions.at(_id); } bool CSECodeGenerator::canBeRemoved(ExpressionClasses::Id _element, ExpressionClasses::Id _result) @@ -401,22 +507,23 @@ bool CSECodeGenerator::removeStackTopIfPossible() void CSECodeGenerator::appendDup(int _fromPosition) { + assertThrow(_fromPosition != c_invalidPosition, OptimizerException, ""); int nr = 1 + m_stackHeight - _fromPosition; assertThrow(nr <= 16, StackTooDeepException, "Stack too deep."); assertThrow(1 <= nr, OptimizerException, "Invalid stack access."); - m_generatedItems.push_back(AssemblyItem(dupInstruction(nr))); - m_stackHeight++; + appendItem(AssemblyItem(dupInstruction(nr))); m_stack[m_stackHeight] = m_stack[_fromPosition]; } -void CSECodeGenerator::appendSwapOrRemove(int _fromPosition) +void CSECodeGenerator::appendOrRemoveSwap(int _fromPosition) { + assertThrow(_fromPosition != c_invalidPosition, OptimizerException, ""); if (_fromPosition == m_stackHeight) return; int nr = m_stackHeight - _fromPosition; assertThrow(nr <= 16, StackTooDeepException, "Stack too deep."); assertThrow(1 <= nr, OptimizerException, "Invalid stack access."); - m_generatedItems.push_back(AssemblyItem(swapInstruction(nr))); + appendItem(AssemblyItem(swapInstruction(nr))); // The value of a class can be present in multiple locations on the stack. We only update the // "canonical" one that is tracked by m_classPositions if (m_classPositions[m_stack[m_stackHeight]] == m_stackHeight) diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index 331a7642d..c3e291afb 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -25,6 +25,8 @@ #include #include +#include +#include #include #include #include @@ -44,9 +46,9 @@ using AssemblyItems = std::vector; * known to be equal only once. * * The general workings are that for each assembly item that is fed into the eliminator, an - * equivalence class is derived from the operation and the equivalence class of its arguments and - * it is assigned to the next sequence number of a stack item. DUPi, SWAPi and some arithmetic - * instructions are used to infer equivalences while these classes are determined. + * equivalence class is derived from the operation and the equivalence class of its arguments. + * DUPi, SWAPi and some arithmetic instructions are used to infer equivalences while these + * classes are determined. * * When the list of optimized items is requested, they are generated in a bottom-up fashion, * adding code for equivalence classes that were not yet computed. @@ -54,6 +56,23 @@ using AssemblyItems = std::vector; class CommonSubexpressionEliminator { public: + struct StorageWriteOperation + { + StorageWriteOperation( + ExpressionClasses::Id _slot, + unsigned _sequenceNumber, + ExpressionClasses::Id _expression + ): slot(_slot), sequenceNumber(_sequenceNumber), expression(_expression) {} + bool operator<(StorageWriteOperation const& _other) const + { + return std::tie(slot, sequenceNumber, expression) < + std::tie(_other.slot, _other.sequenceNumber, _other.expression); + } + ExpressionClasses::Id slot; + unsigned sequenceNumber; + ExpressionClasses::Id expression; + }; + /// 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. template @@ -65,7 +84,7 @@ public: /// Streams debugging information to @a _out. std::ostream& stream( std::ostream& _out, - std::map _currentStack = std::map(), + std::map _initialStack = std::map(), std::map _targetStack = std::map() ) const; @@ -85,10 +104,23 @@ private: /// (must not be positive). ExpressionClasses::Id initialStackElement(int _stackHeight); + /// Increments the sequence number, deletes all storage information that might be overwritten + /// and stores the new value at the given slot. + void storeInStorage(ExpressionClasses::Id _slot, ExpressionClasses::Id _value); + /// Retrieves the current value at the given slot in storage or creates a new special sload class. + ExpressionClasses::Id loadFromStorage(ExpressionClasses::Id _slot); + /// Current stack height, can be negative. int m_stackHeight = 0; /// Current stack layout, mapping stack height -> equivalence class std::map m_stackElements; + /// Current sequence number, this is incremented with each modification to storage. + unsigned m_sequenceNumber = 1; + /// Knowledge about storage content. + std::map m_storageContent; + /// Keeps information about which storage slots were written to at which sequence number with + /// what SSTORE instruction. + std::set m_storageWrites; /// Structure containing the classes of equivalent expressions. ExpressionClasses m_expressionClasses; }; @@ -114,14 +146,19 @@ struct SemanticInformation class CSECodeGenerator { public: - CSECodeGenerator(ExpressionClasses const& _expressionClasses): - m_expressionClasses(_expressionClasses) + using StorageWriteOperation = CommonSubexpressionEliminator::StorageWriteOperation; + + CSECodeGenerator( + ExpressionClasses& _expressionClasses, + std::set const& _storageWrites + ): + m_expressionClasses(_expressionClasses), + m_storageWrites(_storageWrites) {} /// @returns the assembly items generated from the given requirements /// @param _initialStack current contents of the stack (up to stack height of zero) /// @param _targetStackContents final contents of the stack, by stack height relative to initial - /// @param _equivalenceClasses equivalence classes as expressions of how to compute them /// @note should only be called once on each object. AssemblyItems generateCode( std::map const& _initialStack, @@ -133,8 +170,13 @@ private: void addDependencies(ExpressionClasses::Id _c); /// Produce code that generates the given element if it is not yet present. - /// @returns the stack position of the element. - int generateClassElement(ExpressionClasses::Id _c); + /// @returns the stack position of the element or c_invalidPosition if it does not actually + /// generate a value on the stack. + /// @param _allowSequenced indicates that sequence-constrained operations are allowed + int generateClassElement(ExpressionClasses::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(ExpressionClasses::Id _id) const; /// @returns true if @a _element can be removed - in general or, if given, while computing @a _result. bool canBeRemoved(ExpressionClasses::Id _element, ExpressionClasses::Id _result = ExpressionClasses::Id(-1)); @@ -146,7 +188,7 @@ private: void appendDup(int _fromPosition); /// Appends a swap instruction to m_generatedItems to retrieve the element at the given stack position. /// @note this might also remove the last item if it exactly the same swap instruction. - void appendSwapOrRemove(int _fromPosition); + void appendOrRemoveSwap(int _fromPosition); /// Appends the given assembly item. void appendItem(AssemblyItem const& _item); @@ -163,7 +205,10 @@ private: std::map m_classPositions; /// The actual eqivalence class items and how to compute them. - ExpressionClasses const& m_expressionClasses; + ExpressionClasses& m_expressionClasses; + /// Keeps information about which storage slots were written to at which sequence number with + /// what SSTORE instruction. + std::set const& m_storageWrites; /// The set of equivalence classes that should be present on the stack at the end. std::set m_finalClasses; }; diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index b43b54113..298d11dbd 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -39,16 +39,22 @@ bool ExpressionClasses::Expression::operator<(ExpressionClasses::Expression cons { auto type = item->type(); auto otherType = _other.item->type(); - return std::tie(type, item->data(), arguments) < - std::tie(otherType, _other.item->data(), _other.arguments); + return std::tie(type, item->data(), arguments, sequenceNumber) < + std::tie(otherType, _other.item->data(), _other.arguments, _other.sequenceNumber); } -ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids const& _arguments) +ExpressionClasses::Id ExpressionClasses::find( + AssemblyItem const& _item, + Ids const& _arguments, + bool _copyItem, + unsigned _sequenceNumber +) { Expression exp; exp.id = Id(-1); exp.item = &_item; exp.arguments = _arguments; + exp.sequenceNumber = _sequenceNumber; if (SemanticInformation::isCommutativeOperation(_item)) sort(exp.arguments.begin(), exp.arguments.end()); @@ -58,9 +64,8 @@ ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids con if (!(e < exp || exp < e)) return e.id; - if (SemanticInformation::isDupInstruction(_item)) + if (_copyItem) { - // Special item that refers to values pre-existing on the stack m_spareAssemblyItem.push_back(make_shared(_item)); exp.item = m_spareAssemblyItem.back().get(); } @@ -74,6 +79,17 @@ ExpressionClasses::Id ExpressionClasses::find(AssemblyItem const& _item, Ids con return exp.id; } +bool ExpressionClasses::knownToBeDifferent(ExpressionClasses::Id _a, ExpressionClasses::Id _b) +{ + // Try to simplify "_a - _b" and return true iff the value is a non-zero constant. + //@todo we could try to cache this information + map matchGroups; + Pattern constant(Push); + constant.setMatchGroup(1, matchGroups); + Id difference = find(Instruction::SUB, {_a, _b}); + return constant.matches(representative(difference), *this) && constant.d() != u256(0); +} + string ExpressionClasses::fullDAGToString(ExpressionClasses::Id _id) const { Expression const& expr = representative(_id); @@ -189,27 +205,46 @@ Rules::Rules() // Moving constants to the outside, order matters here! // we need actions that return expressions (or patterns?) here, and we need also reversed rules // (X+A)+B -> X+(A+B) - m_rules.push_back({ + m_rules += vector>>{{ {op, {{op, {X, A}}, B}}, [=]() -> Pattern { return {op, {X, fun(A.d(), B.d())}}; } - }); + }, { // X+(Y+A) -> (X+Y)+A - m_rules.push_back({ {op, {{op, {X, A}}, Y}}, [=]() -> Pattern { return {op, {{op, {X, Y}}, A}}; } - }); + }, { // For now, we still need explicit commutativity for the inner pattern - m_rules.push_back({ {op, {{op, {A, X}}, B}}, [=]() -> Pattern { return {op, {X, fun(A.d(), B.d())}}; } - }); - m_rules.push_back({ + }, { {op, {{op, {A, X}}, Y}}, [=]() -> Pattern { return {op, {{op, {X, Y}}, A}}; } - }); + }}; + } + // move constants across subtractions + m_rules += vector>>{ + { + // X - A -> X + (-A) + {Instruction::SUB, {X, A}}, + [=]() -> Pattern { return {Instruction::ADD, {X, 0 - A.d()}}; } + }, { + // (X + A) - Y -> (X - Y) + A + {Instruction::SUB, {{Instruction::ADD, {X, A}}, Y}}, + [=]() -> Pattern { return {Instruction::ADD, {{Instruction::SUB, {X, Y}}, A}}; } + }, { + // (A + X) - Y -> (X - Y) + A + {Instruction::SUB, {{Instruction::ADD, {A, X}}, Y}}, + [=]() -> Pattern { return {Instruction::ADD, {{Instruction::SUB, {X, Y}}, A}}; } + }, { + // X - (Y + A) -> (X - Y) + (-A) + {Instruction::SUB, {X, {Instruction::ADD, {Y, A}}}}, + [=]() -> Pattern { return {Instruction::ADD, {{Instruction::SUB, {X, Y}}, 0 - A.d()}}; } + }, { + // X - (A + Y) -> (X - Y) + (-A) + {Instruction::SUB, {X, {Instruction::ADD, {A, Y}}}}, + [=]() -> Pattern { return {Instruction::ADD, {{Instruction::SUB, {X, Y}}, 0 - A.d()}}; } + } }; - - //@todo: (x+8)-3 and other things } ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr, bool _secondRun) @@ -231,7 +266,7 @@ ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr, //cout << ")" << endl; //cout << "with rule " << rule.first.toString() << endl; //ExpressionTemplate t(rule.second()); - //cout << "to" << rule.second().toString() << endl; + //cout << "to " << rule.second().toString() << endl; return rebuildExpression(ExpressionTemplate(rule.second())); } } @@ -254,8 +289,7 @@ ExpressionClasses::Id ExpressionClasses::rebuildExpression(ExpressionTemplate co Ids arguments; for (ExpressionTemplate const& t: _template.arguments) arguments.push_back(rebuildExpression(t)); - m_spareAssemblyItem.push_back(make_shared(_template.item)); - return find(*m_spareAssemblyItem.back(), arguments); + return find(_template.item, arguments); } diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index eda568e23..b7868ebb9 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -52,17 +52,25 @@ public: Id id; AssemblyItem const* item; Ids arguments; + unsigned sequenceNumber; ///< Storage modification sequence, only used for SLOAD/SSTORE instructions. bool operator<(Expression const& _other) const; }; /// Retrieves the id of the expression equivalence class resulting from the given item applied to the /// given classes, might also create a new one. - Id find(AssemblyItem const& _item, Ids const& _arguments = {}); + /// @param _copyItem if true, copies the assembly item to an internal storage instead of just + /// keeping a pointer. + /// The @a _sequenceNumber indicates the current storage access sequence. + Id find(AssemblyItem const& _item, Ids const& _arguments = {}, bool _copyItem = true, unsigned _sequenceNumber = 0); /// @returns the canonical representative of an expression class. Expression const& representative(Id _id) const { return m_representatives.at(_id); } /// @returns the number of classes. Id size() const { return m_representatives.size(); } + /// @returns true if the values of the given classes are known to be different (on every input). + /// @note that this function might still return false for some different inputs. + bool knownToBeDifferent(Id _a, Id _b); + std::string fullDAGToString(Id _id) const; private: diff --git a/test/SolidityOptimizer.cpp b/test/SolidityOptimizer.cpp index 2d5cff7ac..de4cac8fd 100644 --- a/test/SolidityOptimizer.cpp +++ b/test/SolidityOptimizer.cpp @@ -303,6 +303,186 @@ BOOST_AUTO_TEST_CASE(cse_associativity2) checkCSE(input, {Instruction::DUP2, Instruction::DUP2, Instruction::ADD, u256(5), Instruction::ADD}); } +BOOST_AUTO_TEST_CASE(cse_storage) +{ + AssemblyItems input{ + u256(0), + Instruction::SLOAD, + u256(0), + Instruction::SLOAD, + Instruction::ADD, + u256(0), + Instruction::SSTORE + }; + checkCSE(input, { + u256(0), + Instruction::DUP1, + Instruction::SLOAD, + Instruction::DUP1, + Instruction::ADD, + Instruction::SWAP1, + Instruction::SSTORE + }); +} + +BOOST_AUTO_TEST_CASE(cse_noninterleaved_storage) +{ + // two stores to the same location should be replaced by only one store, even if we + // read in the meantime + AssemblyItems input{ + u256(7), + Instruction::DUP2, + Instruction::SSTORE, + Instruction::DUP1, + Instruction::SLOAD, + u256(8), + Instruction::DUP3, + Instruction::SSTORE + }; + checkCSE(input, { + u256(8), + Instruction::DUP2, + Instruction::SSTORE, + u256(7) + }); +} + +BOOST_AUTO_TEST_CASE(cse_interleaved_storage) +{ + // stores and reads to/from two unknown locations, should not optimize away the first store + AssemblyItems input{ + u256(7), + Instruction::DUP2, + Instruction::SSTORE, // store to "DUP1" + Instruction::DUP2, + Instruction::SLOAD, // read from "DUP2", might be equal to "DUP1" + u256(0), + Instruction::DUP3, + Instruction::SSTORE // store different value to "DUP1" + }; + checkCSE(input, input); +} + +BOOST_AUTO_TEST_CASE(cse_interleaved_storage_same_value) +{ + // stores and reads to/from two unknown locations, should not optimize away the first store + // but it should optimize away the second, since we already know the value will be the same + AssemblyItems input{ + u256(7), + Instruction::DUP2, + Instruction::SSTORE, // store to "DUP1" + Instruction::DUP2, + Instruction::SLOAD, // read from "DUP2", might be equal to "DUP1" + u256(6), + u256(1), + Instruction::ADD, + Instruction::DUP3, + Instruction::SSTORE // store same value to "DUP1" + }; + checkCSE(input, { + u256(7), + Instruction::DUP2, + Instruction::SSTORE, + Instruction::DUP2, + Instruction::SLOAD + }); +} + +BOOST_AUTO_TEST_CASE(cse_interleaved_storage_at_known_location) +{ + // stores and reads to/from two known locations, should optimize away the first store, + // because we know that the location is different + AssemblyItems input{ + u256(0x70), + u256(1), + Instruction::SSTORE, // store to 1 + u256(2), + Instruction::SLOAD, // read from 2, is different from 1 + u256(0x90), + u256(1), + Instruction::SSTORE // store different value at 1 + }; + checkCSE(input, { + u256(2), + Instruction::SLOAD, + u256(0x90), + u256(1), + Instruction::SSTORE + }); +} + +BOOST_AUTO_TEST_CASE(cse_interleaved_storage_at_known_location_offset) +{ + // stores and reads to/from two locations which are known to be different, + // should optimize away the first store, because we know that the location is different + AssemblyItems input{ + u256(0x70), + Instruction::DUP2, + u256(1), + Instruction::ADD, + Instruction::SSTORE, // store to "DUP1"+1 + Instruction::DUP1, + u256(2), + Instruction::ADD, + Instruction::SLOAD, // read from "DUP1"+2, is different from "DUP1"+1 + u256(0x90), + Instruction::DUP3, + u256(1), + Instruction::ADD, + Instruction::SSTORE // store different value at "DUP1"+1 + }; + checkCSE(input, { + u256(2), + Instruction::DUP2, + Instruction::ADD, + Instruction::SLOAD, + u256(0x90), + u256(1), + Instruction::DUP4, + Instruction::ADD, + Instruction::SSTORE + }); +} + +BOOST_AUTO_TEST_CASE(cse_deep_stack) +{ + AssemblyItems input{ + Instruction::ADD, + Instruction::SWAP1, + Instruction::POP, + Instruction::SWAP8, + Instruction::POP, + Instruction::SWAP8, + Instruction::POP, + Instruction::SWAP8, + Instruction::SWAP5, + Instruction::POP, + Instruction::POP, + Instruction::POP, + Instruction::POP, + Instruction::POP, + }; + checkCSE(input, { + Instruction::SWAP4, + Instruction::SWAP12, + Instruction::SWAP3, + Instruction::SWAP11, + Instruction::POP, + Instruction::SWAP1, + Instruction::SWAP3, + Instruction::ADD, + Instruction::SWAP8, + Instruction::POP, + Instruction::SWAP6, + Instruction::POP, + Instruction::POP, + Instruction::POP, + Instruction::POP, + Instruction::POP, + Instruction::POP, + }); +} + BOOST_AUTO_TEST_SUITE_END() } From c19229415d0f9704d5c9e6ac11733824a56dec20 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 25 Mar 2015 18:42:18 +0100 Subject: [PATCH 02/13] Some optimizations to the optimizer. --- libevmcore/ExpressionClasses.cpp | 19 ++++++++++--------- libevmcore/ExpressionClasses.h | 3 +++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index 298d11dbd..717379be5 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -59,10 +59,9 @@ ExpressionClasses::Id ExpressionClasses::find( if (SemanticInformation::isCommutativeOperation(_item)) sort(exp.arguments.begin(), exp.arguments.end()); - //@todo store all class members (not only the representatives) in an efficient data structure to search here - for (Expression const& e: m_representatives) - if (!(e < exp || exp < e)) - return e.id; + auto it = m_expressions.find(exp); + if (it != m_expressions.end()) + return it->id; if (_copyItem) { @@ -72,17 +71,19 @@ ExpressionClasses::Id ExpressionClasses::find( ExpressionClasses::Id id = tryToSimplify(exp); if (id < m_representatives.size()) - return id; - - exp.id = m_representatives.size(); - m_representatives.push_back(exp); + exp.id = id; + else + { + exp.id = m_representatives.size(); + m_representatives.push_back(exp); + } + m_expressions.insert(exp); return exp.id; } bool ExpressionClasses::knownToBeDifferent(ExpressionClasses::Id _a, ExpressionClasses::Id _b) { // Try to simplify "_a - _b" and return true iff the value is a non-zero constant. - //@todo we could try to cache this information map matchGroups; Pattern constant(Push); constant.setMatchGroup(1, matchGroups); diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index b7868ebb9..15824f6ad 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -53,6 +53,7 @@ public: AssemblyItem const* item; Ids arguments; unsigned sequenceNumber; ///< Storage modification sequence, only used for SLOAD/SSTORE instructions. + /// Behaves as if this was a tuple of (item->type(), item->data(), arguments, sequenceNumber). bool operator<(Expression const& _other) const; }; @@ -86,6 +87,8 @@ private: /// Expression equivalence class representatives - we only store one item of an equivalence. std::vector m_representatives; + /// All expression ever encountered. + std::set m_expressions; std::vector> m_spareAssemblyItem; }; From 9da4c207a3cdfc65096060f7069a452451ef293a Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Mar 2015 13:03:24 +0100 Subject: [PATCH 03/13] Change to storage operations. --- libevmcore/CommonSubexpressionEliminator.cpp | 81 ++++++++++---------- libevmcore/CommonSubexpressionEliminator.h | 36 ++++----- libevmcore/ExpressionClasses.h | 9 ++- 3 files changed, 63 insertions(+), 63 deletions(-) diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 43d01fc85..6002de758 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -45,7 +45,7 @@ vector CommonSubexpressionEliminator::getOptimizedItems() // Debug info: //stream(cout, initialStackContents, targetStackContents); - return CSECodeGenerator(m_expressionClasses, m_storageWrites).generateCode( + return CSECodeGenerator(m_expressionClasses, m_storeOperations).generateCode( initialStackContents, targetStackContents ); @@ -172,7 +172,7 @@ void CommonSubexpressionEliminator::storeInStorage(ExpressionClasses::Id _slot, storageContents.insert(storageItem); m_storageContent = move(storageContents); ExpressionClasses::Id id = m_expressionClasses.find(Instruction::SSTORE, {_slot, _value}, true, m_sequenceNumber); - m_storageWrites.insert(StorageWriteOperation(_slot, m_sequenceNumber, id)); + m_storeOperations.push_back(StoreOperation(_slot, m_sequenceNumber, id)); m_storageContent[_slot] = _value; // increment a second time so that we get unique sequence numbers for writes m_sequenceNumber ++; @@ -249,6 +249,16 @@ bool SemanticInformation::isSwapInstruction(AssemblyItem const& _item) return Instruction::SWAP1 <= _item.instruction() && _item.instruction() <= Instruction::SWAP16; } +CSECodeGenerator::CSECodeGenerator( + ExpressionClasses& _expressionClasses, + vector const& _storeOperations +): + m_expressionClasses(_expressionClasses) +{ + for (auto const& store: _storeOperations) + m_storeOperations[store.slot].push_back(store); +} + AssemblyItems CSECodeGenerator::generateCode( map const& _initialStack, map const& _targetStackContents @@ -261,35 +271,26 @@ AssemblyItems CSECodeGenerator::generateCode( // @todo: provide information about the positions of copies of class elements - // generate the dependency graph starting from final storage writes and target stack contents - for (auto it = m_storageWrites.begin(); it != m_storageWrites.end();) - { - auto next = it; - ++next; - if (next == m_storageWrites.end() || next->slot != it->slot) - // last write to that storage slot - addDependencies(it->expression); - it = next; - } + // generate the dependency graph starting from final storage and memory writes and target stack contents + for (auto const& p: m_storeOperations) + addDependencies(p.second.back().expression); for (auto const& targetItem: _targetStackContents) { m_finalClasses.insert(targetItem.second); addDependencies(targetItem.second); } - // Perform all operations on storage in order, if they are needed. - //@todo use better data structures to optimize these loops - unsigned maxSequenceNumber = 1; - for (StorageWriteOperation const& op: m_storageWrites) - maxSequenceNumber = max(maxSequenceNumber, op.sequenceNumber + 1); - for (unsigned sequenceNumber = 1; sequenceNumber <= maxSequenceNumber; ++sequenceNumber) - for (auto const& depPair: m_neededBy) - for (ExpressionClasses::Id const& id: {depPair.first, depPair.second}) - if ( - m_expressionClasses.representative(id).sequenceNumber == sequenceNumber && - !m_classPositions.count(id) - ) - generateClassElement(id, true); + // store all needed sequenced expressions + set> sequencedExpressions; + for (auto const& p: m_neededBy) + for (auto id: {p.first, p.second}) + if (unsigned seqNr = m_expressionClasses.representative(id).sequenceNumber) + sequencedExpressions.insert(make_pair(seqNr, id)); + + // 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, true); // generate the target stack elements for (auto const& targetItem: _targetStackContents) @@ -340,23 +341,25 @@ void CSECodeGenerator::addDependencies(ExpressionClasses::Id _c) if (expr.item->type() == Operation && expr.item->instruction() == Instruction::SLOAD) { // this loads an unknown value from storage and thus, in addition to its arguments, depends - // on all SSTORE operations to addresses where we do not know that they are different that - // occur before this SLOAD + // on all store operations to addresses where we do not know that they are different that + // occur before this load ExpressionClasses::Id slotToLoadFrom = expr.arguments.at(0); - for (auto it = m_storageWrites.begin(); it != m_storageWrites.end();) + for (auto const& p: m_storeOperations) { - auto next = it; - ++next; - // note that SSTORE and SLOAD never have the same sequence number - if (it->sequenceNumber < expr.sequenceNumber && - !m_expressionClasses.knownToBeDifferent(it->slot, slotToLoadFrom) && - (next == m_storageWrites.end() || next->sequenceNumber > expr.sequenceNumber) + ExpressionClasses::Id slot = p.first; + StoreOperations const& storeOps = p.second; + if ( + m_expressionClasses.knownToBeDifferent(slot, slotToLoadFrom) || + storeOps.front().sequenceNumber > expr.sequenceNumber ) - { - addDependencies(it->expression); - m_neededBy.insert(make_pair(it->expression, _c)); - } - it = next; + continue; + // note that store and load never have the same sequence number + ExpressionClasses::Id latestStore = storeOps.front().expression; + for (auto it = ++storeOps.begin(); it != storeOps.end(); ++it) + if (it->sequenceNumber < expr.sequenceNumber) + latestStore = it->expression; + addDependencies(latestStore); + m_neededBy.insert(make_pair(latestStore, _c)); } } } diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index c3e291afb..c4d960d91 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -56,18 +56,13 @@ using AssemblyItems = std::vector; class CommonSubexpressionEliminator { public: - struct StorageWriteOperation + struct StoreOperation { - StorageWriteOperation( + StoreOperation( ExpressionClasses::Id _slot, unsigned _sequenceNumber, ExpressionClasses::Id _expression ): slot(_slot), sequenceNumber(_sequenceNumber), expression(_expression) {} - bool operator<(StorageWriteOperation const& _other) const - { - return std::tie(slot, sequenceNumber, expression) < - std::tie(_other.slot, _other.sequenceNumber, _other.expression); - } ExpressionClasses::Id slot; unsigned sequenceNumber; ExpressionClasses::Id expression; @@ -114,13 +109,13 @@ private: int m_stackHeight = 0; /// Current stack layout, mapping stack height -> equivalence class std::map m_stackElements; - /// Current sequence number, this is incremented with each modification to storage. + /// Current sequence number, this is incremented with each modification to storage or memory. unsigned m_sequenceNumber = 1; /// Knowledge about storage content. std::map m_storageContent; - /// Keeps information about which storage slots were written to at which sequence number with - /// what SSTORE instruction. - std::set m_storageWrites; + /// Keeps information about which storage or memory slots were written to at which sequence + /// number with what instruction. + std::vector m_storeOperations; /// Structure containing the classes of equivalent expressions. ExpressionClasses m_expressionClasses; }; @@ -146,15 +141,12 @@ struct SemanticInformation class CSECodeGenerator { public: - using StorageWriteOperation = CommonSubexpressionEliminator::StorageWriteOperation; + using StoreOperation = CommonSubexpressionEliminator::StoreOperation; + using StoreOperations = std::vector; - CSECodeGenerator( - ExpressionClasses& _expressionClasses, - std::set const& _storageWrites - ): - m_expressionClasses(_expressionClasses), - m_storageWrites(_storageWrites) - {} + /// Initializes the code generator with the given classes and store operations. + /// The store operations have to be sorted ascendingly by sequence number. + CSECodeGenerator(ExpressionClasses& _expressionClasses, StoreOperations const& _storeOperations); /// @returns the assembly items generated from the given requirements /// @param _initialStack current contents of the stack (up to stack height of zero) @@ -206,9 +198,9 @@ private: /// The actual eqivalence class items and how to compute them. ExpressionClasses& m_expressionClasses; - /// Keeps information about which storage slots were written to at which sequence number with - /// what SSTORE instruction. - std::set const& m_storageWrites; + /// Keeps information about which storage or memory slots were written to by which operations. + /// The operations are sorted ascendingly by sequence number. + std::map m_storeOperations; /// The set of equivalence classes that should be present on the stack at the end. std::set m_finalClasses; }; diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index 15824f6ad..d76fd693d 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -61,8 +61,13 @@ public: /// given classes, might also create a new one. /// @param _copyItem if true, copies the assembly item to an internal storage instead of just /// keeping a pointer. - /// The @a _sequenceNumber indicates the current storage access sequence. - Id find(AssemblyItem const& _item, Ids const& _arguments = {}, bool _copyItem = true, unsigned _sequenceNumber = 0); + /// The @a _sequenceNumber indicates the current storage or memory access sequence. + Id find( + AssemblyItem const& _item, + Ids const& _arguments = {}, + bool _copyItem = true, + unsigned _sequenceNumber = 0 + ); /// @returns the canonical representative of an expression class. Expression const& representative(Id _id) const { return m_representatives.at(_id); } /// @returns the number of classes. From d6a611429f39d726672db6331db86e7d5306a05d Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Mar 2015 15:50:50 +0100 Subject: [PATCH 04/13] Optimizer for memory. --- libevmcore/CommonSubexpressionEliminator.cpp | 70 +++++++++++++++++--- libevmcore/CommonSubexpressionEliminator.h | 16 ++++- libevmcore/ExpressionClasses.cpp | 13 ++++ libevmcore/ExpressionClasses.h | 2 + test/SolidityOptimizer.cpp | 53 +++++++++++++++ 5 files changed, 142 insertions(+), 12 deletions(-) diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 6002de758..1d2a71836 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -121,6 +121,10 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) storeInStorage(arguments[0], arguments[1]); else if (_item.instruction() == Instruction::SLOAD) setStackElement(m_stackHeight + _item.deposit(), loadFromStorage(arguments[0])); + else if (_item.instruction() == Instruction::MSTORE) + storeInMemory(arguments[0], arguments[1]); + else if (_item.instruction() == Instruction::MLOAD) + setStackElement(m_stackHeight + _item.deposit(), loadFromMemory(arguments[0])); else setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments, false)); } @@ -172,7 +176,7 @@ void CommonSubexpressionEliminator::storeInStorage(ExpressionClasses::Id _slot, storageContents.insert(storageItem); m_storageContent = move(storageContents); ExpressionClasses::Id id = m_expressionClasses.find(Instruction::SSTORE, {_slot, _value}, true, m_sequenceNumber); - m_storeOperations.push_back(StoreOperation(_slot, m_sequenceNumber, id)); + m_storeOperations.push_back(StoreOperation(StoreOperation::Storage, _slot, m_sequenceNumber, id)); m_storageContent[_slot] = _value; // increment a second time so that we get unique sequence numbers for writes m_sequenceNumber ++; @@ -186,6 +190,33 @@ ExpressionClasses::Id CommonSubexpressionEliminator::loadFromStorage(ExpressionC return m_storageContent[_slot] = m_expressionClasses.find(Instruction::SLOAD, {_slot}, true, m_sequenceNumber); } +void CommonSubexpressionEliminator::storeInMemory(ExpressionClasses::Id _slot, ExpressionClasses::Id _value) +{ + if (m_memoryContent.count(_slot) && m_memoryContent[_slot] == _value) + // do not execute the store if we know that the value is already there + return; + m_sequenceNumber ++; + decltype(m_memoryContent) memoryContents; + // copy over values at points where we know that they are different from _slot by at least 32 + for (auto const& memoryItem: m_memoryContent) + if (m_expressionClasses.knownToBeDifferentBy32(memoryItem.first, _slot)) + memoryContents.insert(memoryItem); + m_memoryContent = move(memoryContents); + ExpressionClasses::Id id = m_expressionClasses.find(Instruction::MSTORE, {_slot, _value}, true, m_sequenceNumber); + m_storeOperations.push_back(StoreOperation(StoreOperation::Memory, _slot, m_sequenceNumber, id)); + m_memoryContent[_slot] = _value; + // increment a second time so that we get unique sequence numbers for writes + m_sequenceNumber ++; +} + +ExpressionClasses::Id CommonSubexpressionEliminator::loadFromMemory(ExpressionClasses::Id _slot) +{ + if (m_memoryContent.count(_slot)) + return m_memoryContent.at(_slot); + else + return m_memoryContent[_slot] = m_expressionClasses.find(Instruction::MLOAD, {_slot}, true, m_sequenceNumber); +} + bool SemanticInformation::breaksBasicBlock(AssemblyItem const& _item) { switch (_item.type()) @@ -208,9 +239,19 @@ bool SemanticInformation::breaksBasicBlock(AssemblyItem const& _item) return false; if (_item.instruction() == Instruction::GAS || _item.instruction() == Instruction::PC) return true; // GAS and PC assume a specific order of opcodes + if (_item.instruction() == Instruction::MSIZE) + return true; // msize is modified already by memory access, avoid that for now + if (_item.instruction() == Instruction::SHA3) + return true; //@todo: we have to compare sha3's not based on their memory addresses but on the memory content. InstructionInfo info = instructionInfo(_item.instruction()); if (_item.instruction() == Instruction::SSTORE) return false; + if (_item.instruction() == Instruction::MSTORE) + return false; + //@todo: We do not handle the following memory instructions for now: + // calldatacopy, codecopy, extcodecopy, mstore8, + // msize (not that msize also depends on memory read access) + // the second requirement will be lifted once it is implemented return info.sideEffects || info.args > 2; } @@ -256,7 +297,7 @@ CSECodeGenerator::CSECodeGenerator( m_expressionClasses(_expressionClasses) { for (auto const& store: _storeOperations) - m_storeOperations[store.slot].push_back(store); + m_storeOperations[make_pair(store.target, store.slot)].push_back(store); } AssemblyItems CSECodeGenerator::generateCode( @@ -332,25 +373,34 @@ void CSECodeGenerator::addDependencies(ExpressionClasses::Id _c) { if (m_neededBy.count(_c)) return; // we already computed the dependencies for _c - ExpressionClasses::Expression const& expr = m_expressionClasses.representative(_c); + ExpressionClasses::Expression expr = m_expressionClasses.representative(_c); for (ExpressionClasses::Id argument: expr.arguments) { addDependencies(argument); m_neededBy.insert(make_pair(argument, _c)); } - if (expr.item->type() == Operation && expr.item->instruction() == Instruction::SLOAD) + if (expr.item->type() == Operation && ( + expr.item->instruction() == Instruction::SLOAD || + expr.item->instruction() == Instruction::MLOAD + )) { - // this loads an unknown value from storage and thus, in addition to its arguments, depends - // on all store operations to addresses where we do not know that they are different that - // occur before this load + // this loads an unknown value from storage or memory and thus, in addition to its + // arguments, depends on all store operations to addresses where we do not know that + // they are different that occur before this load + StoreOperation::Target target = expr.item->instruction() == Instruction::SLOAD ? + StoreOperation::Storage : StoreOperation::Memory; ExpressionClasses::Id slotToLoadFrom = expr.arguments.at(0); for (auto const& p: m_storeOperations) { - ExpressionClasses::Id slot = p.first; + if (p.first.first != target) + continue; + ExpressionClasses::Id slot = p.first.second; StoreOperations const& storeOps = p.second; + if (storeOps.front().sequenceNumber > expr.sequenceNumber) + continue; if ( - m_expressionClasses.knownToBeDifferent(slot, slotToLoadFrom) || - storeOps.front().sequenceNumber > expr.sequenceNumber + (target == StoreOperation::Memory && m_expressionClasses.knownToBeDifferentBy32(slot, slotToLoadFrom)) || + (target == StoreOperation::Storage && m_expressionClasses.knownToBeDifferent(slot, slotToLoadFrom)) ) continue; // note that store and load never have the same sequence number diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index c4d960d91..03f8cf987 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -58,11 +58,14 @@ class CommonSubexpressionEliminator public: struct StoreOperation { + enum Target { Memory, Storage }; StoreOperation( + Target _target, ExpressionClasses::Id _slot, unsigned _sequenceNumber, ExpressionClasses::Id _expression - ): slot(_slot), sequenceNumber(_sequenceNumber), expression(_expression) {} + ): target(_target), slot(_slot), sequenceNumber(_sequenceNumber), expression(_expression) {} + Target target; ExpressionClasses::Id slot; unsigned sequenceNumber; ExpressionClasses::Id expression; @@ -104,6 +107,12 @@ private: void storeInStorage(ExpressionClasses::Id _slot, ExpressionClasses::Id _value); /// Retrieves the current value at the given slot in storage or creates a new special sload class. ExpressionClasses::Id loadFromStorage(ExpressionClasses::Id _slot); + /// Increments the sequence number, deletes all memory information that might be overwritten + /// and stores the new value at the given slot. + void storeInMemory(ExpressionClasses::Id _slot, ExpressionClasses::Id _value); + /// Retrieves the current value at the given slot in memory or creates a new special mload class. + ExpressionClasses::Id loadFromMemory(ExpressionClasses::Id _slot); + /// Current stack height, can be negative. int m_stackHeight = 0; @@ -113,6 +122,9 @@ private: unsigned m_sequenceNumber = 1; /// Knowledge about storage content. std::map m_storageContent; + /// Knowledge about memory content. Keys are memory addresses, note that the values overlap + /// and are not contained here if they are not completely known. + std::map m_memoryContent; /// Keeps information about which storage or memory slots were written to at which sequence /// number with what instruction. std::vector m_storeOperations; @@ -200,7 +212,7 @@ private: ExpressionClasses& m_expressionClasses; /// Keeps information about which storage or memory slots were written to by which operations. /// The operations are sorted ascendingly by sequence number. - std::map m_storeOperations; + std::map, StoreOperations> m_storeOperations; /// The set of equivalence classes that should be present on the stack at the end. std::set m_finalClasses; }; diff --git a/libevmcore/ExpressionClasses.cpp b/libevmcore/ExpressionClasses.cpp index 717379be5..1c7a79e6b 100644 --- a/libevmcore/ExpressionClasses.cpp +++ b/libevmcore/ExpressionClasses.cpp @@ -91,6 +91,19 @@ bool ExpressionClasses::knownToBeDifferent(ExpressionClasses::Id _a, ExpressionC return constant.matches(representative(difference), *this) && constant.d() != u256(0); } +bool ExpressionClasses::knownToBeDifferentBy32(ExpressionClasses::Id _a, ExpressionClasses::Id _b) +{ + // Try to simplify "_a - _b" and return true iff the value is at least 32 away from zero. + map matchGroups; + Pattern constant(Push); + constant.setMatchGroup(1, matchGroups); + Id difference = find(Instruction::SUB, {_a, _b}); + if (!constant.matches(representative(difference), *this)) + return false; + // forbidden interval is ["-31", 31] + return constant.d() + 31 > u256(62); +} + string ExpressionClasses::fullDAGToString(ExpressionClasses::Id _id) const { Expression const& expr = representative(_id); diff --git a/libevmcore/ExpressionClasses.h b/libevmcore/ExpressionClasses.h index d76fd693d..5179845f6 100644 --- a/libevmcore/ExpressionClasses.h +++ b/libevmcore/ExpressionClasses.h @@ -76,6 +76,8 @@ public: /// @returns true if the values of the given classes are known to be different (on every input). /// @note that this function might still return false for some different inputs. bool knownToBeDifferent(Id _a, Id _b); + /// Similar to @a knownToBeDifferent but require that abs(_a - b) >= 32. + bool knownToBeDifferentBy32(Id _a, Id _b); std::string fullDAGToString(Id _id) const; diff --git a/test/SolidityOptimizer.cpp b/test/SolidityOptimizer.cpp index de4cac8fd..cf35536e6 100644 --- a/test/SolidityOptimizer.cpp +++ b/test/SolidityOptimizer.cpp @@ -444,6 +444,59 @@ BOOST_AUTO_TEST_CASE(cse_interleaved_storage_at_known_location_offset) }); } +BOOST_AUTO_TEST_CASE(cse_interleaved_memory_at_known_location_offset) +{ + // stores and reads to/from two locations which are known to be different, + // should not optimize away the first store, because the location overlaps with the load, + // but it should optimize away the second, because we know that the location is different by 32 + AssemblyItems input{ + u256(0x50), + Instruction::DUP2, + u256(2), + Instruction::ADD, + Instruction::MSTORE, // ["DUP1"+2] = 0x50 + u256(0x60), + Instruction::DUP2, + u256(32), + Instruction::ADD, + Instruction::MSTORE, // ["DUP1"+32] = 0x60 + Instruction::DUP1, + Instruction::MLOAD, // read from "DUP1" + u256(0x70), + Instruction::DUP3, + u256(32), + Instruction::ADD, + Instruction::MSTORE, // ["DUP1"+32] = 0x70 + u256(0x80), + Instruction::DUP3, + u256(2), + Instruction::ADD, + Instruction::MSTORE, // ["DUP1"+2] = 0x80 + }; + // If the actual code changes too much, we could also simply check that the output contains + // exactly 3 MSTORE and exactly 1 MLOAD instruction. + checkCSE(input, { + u256(0x50), + u256(2), + Instruction::DUP3, + Instruction::ADD, + Instruction::SWAP1, + Instruction::DUP2, + Instruction::MSTORE, // ["DUP1"+2] = 0x50 + Instruction::DUP2, + Instruction::MLOAD, // read from "DUP1" + u256(0x70), + u256(32), + Instruction::DUP5, + Instruction::ADD, + Instruction::MSTORE, // ["DUP1"+32] = 0x70 + u256(0x80), + Instruction::SWAP1, + Instruction::SWAP2, + Instruction::MSTORE // ["DUP1"+2] = 0x80 + }); +} + BOOST_AUTO_TEST_CASE(cse_deep_stack) { AssemblyItems input{ From b1ea943975e0986e1711cce2a8b3d3ad41f3ae10 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 27 Mar 2015 00:24:31 +0100 Subject: [PATCH 05/13] Optimize breaking item. --- libevmcore/Assembly.cpp | 15 +------- libevmcore/CommonSubexpressionEliminator.cpp | 39 ++++++++++++++++++-- libevmcore/CommonSubexpressionEliminator.h | 11 +++++- test/SolidityOptimizer.cpp | 32 ++++++++++++++++ 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/libevmcore/Assembly.cpp b/libevmcore/Assembly.cpp index abe932282..0301c9325 100644 --- a/libevmcore/Assembly.cpp +++ b/libevmcore/Assembly.cpp @@ -187,18 +187,7 @@ Assembly& Assembly::optimise(bool _enable) { if (!_enable) return *this; - std::vector>> rules = - { - { { Push, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - { { PushTag, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - { { PushString, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - { { PushSub, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - { { PushSubSize, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - { { PushProgramSize, Instruction::POP }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - { { Push, PushTag, Instruction::JUMPI }, [](AssemblyItemsConstRef m) -> AssemblyItems { if (m[0].data()) return { m[1], Instruction::JUMP }; else return {}; } }, - { { Instruction::ISZERO, Instruction::ISZERO }, [](AssemblyItemsConstRef) -> AssemblyItems { return {}; } }, - }; - + std::vector>> rules; // jump to next instruction rules.push_back({ { PushTag, Instruction::JUMP, Tag }, [](AssemblyItemsConstRef m) -> AssemblyItems { if (m[0].m_data == m[2].m_data) return {m[2]}; else return m.toVector(); }}); @@ -235,8 +224,6 @@ Assembly& Assembly::optimise(bool _enable) *orig = move(*moveIter); iter = m_items.erase(orig, iter); } - if (iter != m_items.end()) - ++iter; } for (unsigned i = 0; i < m_items.size(); ++i) diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index 1d2a71836..cc2fdc8bd 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -32,6 +32,8 @@ using namespace dev::eth; vector CommonSubexpressionEliminator::getOptimizedItems() { + optimizeBreakingItem(); + map initialStackContents; map targetStackContents; int minHeight = m_stackHeight + 1; @@ -45,10 +47,13 @@ vector CommonSubexpressionEliminator::getOptimizedItems() // Debug info: //stream(cout, initialStackContents, targetStackContents); - return CSECodeGenerator(m_expressionClasses, m_storeOperations).generateCode( + AssemblyItems items = CSECodeGenerator(m_expressionClasses, m_storeOperations).generateCode( initialStackContents, targetStackContents ); + if (m_breakingItem) + items.push_back(*m_breakingItem); + return items; } ostream& CommonSubexpressionEliminator::stream( @@ -91,12 +96,12 @@ ostream& CommonSubexpressionEliminator::stream( return _out; } -void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) +void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item, bool _copyItem) { if (_item.type() != Operation) { assertThrow(_item.deposit() == 1, InvalidDeposit, ""); - setStackElement(++m_stackHeight, m_expressionClasses.find(_item, {}, false)); + setStackElement(++m_stackHeight, m_expressionClasses.find(_item, {}, _copyItem)); } else { @@ -126,12 +131,38 @@ void CommonSubexpressionEliminator::feedItem(AssemblyItem const& _item) else if (_item.instruction() == Instruction::MLOAD) setStackElement(m_stackHeight + _item.deposit(), loadFromMemory(arguments[0])); else - setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments, false)); + setStackElement(m_stackHeight + _item.deposit(), m_expressionClasses.find(_item, arguments, _copyItem)); } m_stackHeight += _item.deposit(); } } +void CommonSubexpressionEliminator::optimizeBreakingItem() +{ + if (!m_breakingItem || *m_breakingItem != AssemblyItem(Instruction::JUMPI)) + return; + + using Id = ExpressionClasses::Id; + static AssemblyItem s_jump = Instruction::JUMP; + + Id condition = stackElement(m_stackHeight - 1); + Id zero = m_expressionClasses.find(u256(0)); + if (m_expressionClasses.knownToBeDifferent(condition, zero)) + { + feedItem(Instruction::SWAP1, true); + feedItem(Instruction::POP, true); + m_breakingItem = &s_jump; + return; + } + Id negatedCondition = m_expressionClasses.find(Instruction::ISZERO, {condition}); + if (m_expressionClasses.knownToBeDifferent(negatedCondition, zero)) + { + feedItem(Instruction::POP, true); + feedItem(Instruction::POP, true); + m_breakingItem = nullptr; + } +} + void CommonSubexpressionEliminator::setStackElement(int _stackHeight, ExpressionClasses::Id _class) { m_stackElements[_stackHeight] = _class; diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index 03f8cf987..09368f7d2 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -88,7 +88,10 @@ public: private: /// Feeds the item into the system for analysis. - void feedItem(AssemblyItem const& _item); + void feedItem(AssemblyItem const& _item, bool _copyItem = false); + + /// Tries to optimize the item that breaks the basic block at the end. + void optimizeBreakingItem(); /// Simplifies the given item using /// Assigns a new equivalence class to the next sequence number of the given stack element. @@ -130,6 +133,10 @@ private: std::vector m_storeOperations; /// Structure containing the classes of equivalent expressions. ExpressionClasses m_expressionClasses; + + /// 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; }; /** @@ -225,6 +232,8 @@ _AssemblyItemIterator CommonSubexpressionEliminator::feedItems( { for (; _iterator != _end && !SemanticInformation::breaksBasicBlock(*_iterator); ++_iterator) feedItem(*_iterator); + if (_iterator != _end) + m_breakingItem = &(*_iterator++); return _iterator; } diff --git a/test/SolidityOptimizer.cpp b/test/SolidityOptimizer.cpp index cf35536e6..e69d5120e 100644 --- a/test/SolidityOptimizer.cpp +++ b/test/SolidityOptimizer.cpp @@ -536,6 +536,38 @@ BOOST_AUTO_TEST_CASE(cse_deep_stack) }); } +BOOST_AUTO_TEST_CASE(cse_jumpi_no_jump) +{ + AssemblyItems input{ + u256(0), + u256(1), + Instruction::DUP2, + AssemblyItem(PushTag, 1), + Instruction::JUMPI + }; + checkCSE(input, { + u256(0), + u256(1) + }); +} + +BOOST_AUTO_TEST_CASE(cse_jumpi_jump) +{ + AssemblyItems input{ + u256(1), + u256(1), + Instruction::DUP2, + AssemblyItem(PushTag, 1), + Instruction::JUMPI + }; + checkCSE(input, { + u256(1), + Instruction::DUP1, + AssemblyItem(PushTag, 1), + Instruction::JUMP + }); +} + BOOST_AUTO_TEST_SUITE_END() } From 9710ce3b7eaa5db9d549950e1514a8eb1bda99fa Mon Sep 17 00:00:00 2001 From: subtly Date: Tue, 31 Mar 2015 10:28:46 +0200 Subject: [PATCH 06/13] Fix #1475 --- alethzero/MainWin.cpp | 1 + libp2p/Host.cpp | 66 ++++++++++++++++++------------------------- test/peer.cpp | 9 ++++++ 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/alethzero/MainWin.cpp b/alethzero/MainWin.cpp index 0de7e2cb8..610fd032d 100644 --- a/alethzero/MainWin.cpp +++ b/alethzero/MainWin.cpp @@ -1774,6 +1774,7 @@ void Main::on_net_triggered() else { ui->downloadView->setDownloadMan(nullptr); + writeSettings(); web3()->stopNetwork(); } } diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index ab39c5c25..5aeeb3f82 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -678,9 +678,6 @@ void Host::disconnectLatePeers() bytes Host::saveNetwork() const { - if (!m_nodeTable) - return bytes(); - std::list peers; { RecursiveGuard l(x_sessions); @@ -692,27 +689,23 @@ bytes Host::saveNetwork() const RLPStream network; int count = 0; + for (auto const& p: peers) { - RecursiveGuard l(x_sessions); - for (auto const& p: peers) + // Only save peers which have connected within 2 days, with properly-advertised port and public IP address + bi::tcp::endpoint endpoint(p.peerEndpoint()); + if (chrono::system_clock::now() - p.m_lastConnected < chrono::seconds(3600 * 48) && endpoint.port() > 0 && endpoint.port() < /*49152*/32768 && p.id != id() && !isPrivateAddress(p.endpoint.udp.address()) && !isPrivateAddress(endpoint.address())) { - // TODO: alpha: Figure out why it ever shares these ports.//p.address.port() >= 30300 && p.address.port() <= 30305 && - // TODO: alpha: if/how to save private addresses - // Only save peers which have connected within 2 days, with properly-advertised port and public IP address - if (chrono::system_clock::now() - p.m_lastConnected < chrono::seconds(3600 * 48) && p.peerEndpoint().port() > 0 && p.peerEndpoint().port() < /*49152*/32768 && p.id != id() && !isPrivateAddress(p.endpoint.udp.address()) && !isPrivateAddress(p.endpoint.tcp.address())) - { - network.appendList(10); - if (p.peerEndpoint().address().is_v4()) - network << p.peerEndpoint().address().to_v4().to_bytes(); - else - network << p.peerEndpoint().address().to_v6().to_bytes(); - // TODO: alpha: replace 0 with trust-state of node - network << p.peerEndpoint().port() << p.id << 0 - << chrono::duration_cast(p.m_lastConnected.time_since_epoch()).count() - << chrono::duration_cast(p.m_lastAttempted.time_since_epoch()).count() - << p.m_failedAttempts << (unsigned)p.m_lastDisconnect << p.m_score << p.m_rating; - count++; - } + // todo: e2e ipv6 support + if (endpoint.address().is_v4()) + network << endpoint.address().to_v4().to_bytes(); + continue; + + network.appendList(10); + network << endpoint.port() << p.id << p.required + << chrono::duration_cast(p.m_lastConnected.time_since_epoch()).count() + << chrono::duration_cast(p.m_lastAttempted.time_since_epoch()).count() + << p.m_failedAttempts << (unsigned)p.m_lastDisconnect << p.m_score << p.m_rating; + count++; } } @@ -731,6 +724,7 @@ bytes Host::saveNetwork() const count++; } } + // else: TODO: use previous configuration if available RLPStream ret(3); ret << dev::p2p::c_protocolVersion << m_alias.secret(); @@ -757,22 +751,14 @@ void Host::restoreNetwork(bytesConstRef _b) for (auto i: r[2]) { + // todo: e2e ipv6 support + // bi::tcp::endpoint(bi::address_v6(i[0].toArray()), i[1].toInt()); + if (i[0].itemCount() != 4) + continue; bi::tcp::endpoint tcp; bi::udp::endpoint udp; - if (i[0].itemCount() == 4) - { - tcp = bi::tcp::endpoint(bi::address_v4(i[0].toArray()), i[1].toInt()); - udp = bi::udp::endpoint(bi::address_v4(i[0].toArray()), i[1].toInt()); - } - else - { - tcp = bi::tcp::endpoint(bi::address_v6(i[0].toArray()), i[1].toInt()); - udp = bi::udp::endpoint(bi::address_v6(i[0].toArray()), i[1].toInt()); - } - - // skip private addresses - // todo: to support private addresseses entries must be stored - // and managed externally by host rather than nodetable. + tcp = bi::tcp::endpoint(bi::address_v4(i[0].toArray()), i[1].toInt()); + udp = bi::udp::endpoint(bi::address_v4(i[0].toArray()), i[1].toInt()); if (isPrivateAddress(tcp.address()) || isPrivateAddress(udp.address())) continue; @@ -783,6 +769,7 @@ void Host::restoreNetwork(bytesConstRef _b) { shared_ptr p = make_shared(); p->id = id; + p->required = i[2].toInt(); p->m_lastConnected = chrono::system_clock::time_point(chrono::seconds(i[4].toInt())); p->m_lastAttempted = chrono::system_clock::time_point(chrono::seconds(i[5].toInt())); p->m_failedAttempts = i[6].toInt(); @@ -792,7 +779,10 @@ void Host::restoreNetwork(bytesConstRef _b) p->endpoint.tcp = tcp; p->endpoint.udp = udp; m_peers[p->id] = p; - m_nodeTable->addNode(*p.get()); + if (p->required) + requirePeer(p->id, p->endpoint.udp.address(), p->endpoint.udp.port()); + else + m_nodeTable->addNode(*p.get()); } } } @@ -801,7 +791,7 @@ void Host::restoreNetwork(bytesConstRef _b) KeyPair Host::networkAlias(bytesConstRef _b) { RLP r(_b); - if (r.itemCount() == 3 && r[0].isInt() && r[0].toInt() == 1) + if (r.itemCount() == 3 && r[0].isInt() && r[0].toInt() == dev::p2p::c_protocolVersion) return move(KeyPair(move(Secret(r[1].toBytes())))); else return move(KeyPair::create()); diff --git a/test/peer.cpp b/test/peer.cpp index 48431504f..904de0e9d 100644 --- a/test/peer.cpp +++ b/test/peer.cpp @@ -57,6 +57,15 @@ BOOST_AUTO_TEST_CASE(host) g_logVerbosity = oldLogVerbosity; } +BOOST_AUTO_TEST_CASE(networkConfig) +{ + Host save("Test", NetworkPreferences(false)); + bytes store(save.saveNetwork()); + + Host restore("Test", NetworkPreferences(false), bytesConstRef(&store)); + BOOST_REQUIRE(save.id() == restore.id()); +} + BOOST_AUTO_TEST_CASE(save_nodes) { std::list hosts; From 8f68a846da4086cdffd78ad2ba887591d87f7752 Mon Sep 17 00:00:00 2001 From: subtly Date: Tue, 31 Mar 2015 11:21:43 +0200 Subject: [PATCH 07/13] nodeid doesn't cast to a bool (bug) --- libp2p/Host.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index 5aeeb3f82..e530c7e66 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -728,7 +728,9 @@ bytes Host::saveNetwork() const RLPStream ret(3); ret << dev::p2p::c_protocolVersion << m_alias.secret(); - ret.appendList(count).appendRaw(network.out(), count); + ret.appendList(count); + if (count) + ret.appendRaw(network.out(), count); return ret.out(); } @@ -769,7 +771,7 @@ void Host::restoreNetwork(bytesConstRef _b) { shared_ptr p = make_shared(); p->id = id; - p->required = i[2].toInt(); + p->required = i[3].toInt(); p->m_lastConnected = chrono::system_clock::time_point(chrono::seconds(i[4].toInt())); p->m_lastAttempted = chrono::system_clock::time_point(chrono::seconds(i[5].toInt())); p->m_failedAttempts = i[6].toInt(); From a0608412acc148889258e51eecde2b882207ba45 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Thu, 26 Mar 2015 21:06:59 +0100 Subject: [PATCH 08/13] removed trailing zeros from rpc results, fixed #1433 --- libweb3jsonrpc/WebThreeStubServerBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libweb3jsonrpc/WebThreeStubServerBase.cpp b/libweb3jsonrpc/WebThreeStubServerBase.cpp index cab986364..7524e7519 100644 --- a/libweb3jsonrpc/WebThreeStubServerBase.cpp +++ b/libweb3jsonrpc/WebThreeStubServerBase.cpp @@ -402,7 +402,7 @@ string WebThreeStubServerBase::eth_getCode(string const& _address, string const& { try { - return toJS(client()->codeAt(jsToAddress(_address), toBlockNumber(_blockNumber)), 1); + return toJS(client()->codeAt(jsToAddress(_address), toBlockNumber(_blockNumber))); } catch (...) { From b1ec65596f7c6532a6d0ea4ac9d70a569a0e3256 Mon Sep 17 00:00:00 2001 From: subtly Date: Tue, 31 Mar 2015 12:26:42 +0200 Subject: [PATCH 09/13] fix bug found in code review. --- libp2p/Host.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index e530c7e66..45ebd6db1 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -692,14 +692,13 @@ bytes Host::saveNetwork() const for (auto const& p: peers) { // Only save peers which have connected within 2 days, with properly-advertised port and public IP address + // todo: e2e ipv6 support bi::tcp::endpoint endpoint(p.peerEndpoint()); + if (!endpoint.address().is_v4()) + continue; + if (chrono::system_clock::now() - p.m_lastConnected < chrono::seconds(3600 * 48) && endpoint.port() > 0 && endpoint.port() < /*49152*/32768 && p.id != id() && !isPrivateAddress(p.endpoint.udp.address()) && !isPrivateAddress(endpoint.address())) { - // todo: e2e ipv6 support - if (endpoint.address().is_v4()) - network << endpoint.address().to_v4().to_bytes(); - continue; - network.appendList(10); network << endpoint.port() << p.id << p.required << chrono::duration_cast(p.m_lastConnected.time_since_epoch()).count() @@ -729,7 +728,7 @@ bytes Host::saveNetwork() const RLPStream ret(3); ret << dev::p2p::c_protocolVersion << m_alias.secret(); ret.appendList(count); - if (count) + if (!!count) ret.appendRaw(network.out(), count); return ret.out(); } From 1d4e61167ffb783781a2106d66d34737a2c5a161 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Tue, 31 Mar 2015 12:43:08 +0200 Subject: [PATCH 10/13] most compact number representation --- libdevcore/CommonJS.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libdevcore/CommonJS.h b/libdevcore/CommonJS.h index cc487d54c..ade089e16 100644 --- a/libdevcore/CommonJS.h +++ b/libdevcore/CommonJS.h @@ -38,7 +38,10 @@ template std::string toJS(FixedHash const& _h) template std::string toJS(boost::multiprecision::number> const& _n) { - return "0x" + toHex(toCompactBigEndian(_n, 1)); + std::string h = toHex(toCompactBigEndian(_n, 1)); + // remove first 0, if it is necessary; + std::string res = h[0] != '0' ? h : h.substr(1); + return "0x" + res; } inline std::string toJS(bytes const& _n, std::size_t _padding = 0) From fc1aac2ef42ff2778c1e9789c9ea48c74adfc4d5 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 31 Mar 2015 14:59:21 +0200 Subject: [PATCH 11/13] style file --- mix/style.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 mix/style.xml diff --git a/mix/style.xml b/mix/style.xml new file mode 100644 index 000000000..75626ac24 --- /dev/null +++ b/mix/style.xml @@ -0,0 +1,19 @@ + + + + + + CodeStyleData + + false + 4 + 2 + false + 4 + + + + DisplayName + Eth + + From 73040eab7e08373d48e67ed36861976b65b44a2c Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 31 Mar 2015 15:07:05 +0200 Subject: [PATCH 12/13] Clarification about spacing. --- CodingStandards.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CodingStandards.txt b/CodingStandards.txt index e1a1b3ba9..a98a74c78 100644 --- a/CodingStandards.txt +++ b/CodingStandards.txt @@ -13,7 +13,7 @@ c. Don't use braces for condition-body one-liners. d. Never place condition bodies on same line as condition. e. Space between first paren and keyword, but *not* following first paren or preceeding final paren. f. No spaces when fewer than intra-expression three parens together; when three or more, space according to clarity. -g. No spaces for subscripting. +g. No spaces for subscripting or unary operators. h. No space before ':' but one after it, except in the ternary operator: one on both sides. i. Space all other operators. j. Braces, when used, always have their own lines and are at same indentation level as "parent" scope. From 01e6801cf5e6b0f9f4cb6f50893e5354caf4137e Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 31 Mar 2015 15:10:21 +0200 Subject: [PATCH 13/13] Style. --- libevmcore/CommonSubexpressionEliminator.cpp | 26 ++++++++++---------- libevmcore/CommonSubexpressionEliminator.h | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/libevmcore/CommonSubexpressionEliminator.cpp b/libevmcore/CommonSubexpressionEliminator.cpp index cc2fdc8bd..47bb5b512 100644 --- a/libevmcore/CommonSubexpressionEliminator.cpp +++ b/libevmcore/CommonSubexpressionEliminator.cpp @@ -199,7 +199,7 @@ void CommonSubexpressionEliminator::storeInStorage(ExpressionClasses::Id _slot, if (m_storageContent.count(_slot) && m_storageContent[_slot] == _value) // do not execute the storage if we know that the value is already there return; - m_sequenceNumber ++; + m_sequenceNumber++; decltype(m_storageContent) storageContents; // copy over values at points where we know that they are different from _slot for (auto const& storageItem: m_storageContent) @@ -210,7 +210,7 @@ void CommonSubexpressionEliminator::storeInStorage(ExpressionClasses::Id _slot, m_storeOperations.push_back(StoreOperation(StoreOperation::Storage, _slot, m_sequenceNumber, id)); m_storageContent[_slot] = _value; // increment a second time so that we get unique sequence numbers for writes - m_sequenceNumber ++; + m_sequenceNumber++; } ExpressionClasses::Id CommonSubexpressionEliminator::loadFromStorage(ExpressionClasses::Id _slot) @@ -226,7 +226,7 @@ void CommonSubexpressionEliminator::storeInMemory(ExpressionClasses::Id _slot, E if (m_memoryContent.count(_slot) && m_memoryContent[_slot] == _value) // do not execute the store if we know that the value is already there return; - m_sequenceNumber ++; + m_sequenceNumber++; decltype(m_memoryContent) memoryContents; // copy over values at points where we know that they are different from _slot by at least 32 for (auto const& memoryItem: m_memoryContent) @@ -237,7 +237,7 @@ void CommonSubexpressionEliminator::storeInMemory(ExpressionClasses::Id _slot, E m_storeOperations.push_back(StoreOperation(StoreOperation::Memory, _slot, m_sequenceNumber, id)); m_memoryContent[_slot] = _value; // increment a second time so that we get unique sequence numbers for writes - m_sequenceNumber ++; + m_sequenceNumber++; } ExpressionClasses::Id CommonSubexpressionEliminator::loadFromMemory(ExpressionClasses::Id _slot) @@ -281,7 +281,7 @@ bool SemanticInformation::breaksBasicBlock(AssemblyItem const& _item) return false; //@todo: We do not handle the following memory instructions for now: // calldatacopy, codecopy, extcodecopy, mstore8, - // msize (not that msize also depends on memory read access) + // msize (note that msize also depends on memory read access) // the second requirement will be lifted once it is implemented return info.sideEffects || info.args > 2; @@ -592,10 +592,10 @@ bool CSECodeGenerator::removeStackTopIfPossible() void CSECodeGenerator::appendDup(int _fromPosition) { assertThrow(_fromPosition != c_invalidPosition, OptimizerException, ""); - int nr = 1 + m_stackHeight - _fromPosition; - assertThrow(nr <= 16, StackTooDeepException, "Stack too deep."); - assertThrow(1 <= nr, OptimizerException, "Invalid stack access."); - appendItem(AssemblyItem(dupInstruction(nr))); + int instructionNum = 1 + m_stackHeight - _fromPosition; + assertThrow(instructionNum <= 16, StackTooDeepException, "Stack too deep."); + assertThrow(1 <= instructionNum, OptimizerException, "Invalid stack access."); + appendItem(AssemblyItem(dupInstruction(instructionNum))); m_stack[m_stackHeight] = m_stack[_fromPosition]; } @@ -604,10 +604,10 @@ void CSECodeGenerator::appendOrRemoveSwap(int _fromPosition) assertThrow(_fromPosition != c_invalidPosition, OptimizerException, ""); if (_fromPosition == m_stackHeight) return; - int nr = m_stackHeight - _fromPosition; - assertThrow(nr <= 16, StackTooDeepException, "Stack too deep."); - assertThrow(1 <= nr, OptimizerException, "Invalid stack access."); - appendItem(AssemblyItem(swapInstruction(nr))); + int instructionNum = m_stackHeight - _fromPosition; + assertThrow(instructionNum <= 16, StackTooDeepException, "Stack too deep."); + assertThrow(1 <= instructionNum, OptimizerException, "Invalid stack access."); + appendItem(AssemblyItem(swapInstruction(instructionNum))); // The value of a class can be present in multiple locations on the stack. We only update the // "canonical" one that is tracked by m_classPositions if (m_classPositions[m_stack[m_stackHeight]] == m_stackHeight) diff --git a/libevmcore/CommonSubexpressionEliminator.h b/libevmcore/CommonSubexpressionEliminator.h index 09368f7d2..a9a0c60a4 100644 --- a/libevmcore/CommonSubexpressionEliminator.h +++ b/libevmcore/CommonSubexpressionEliminator.h @@ -164,7 +164,7 @@ public: using StoreOperations = std::vector; /// Initializes the code generator with the given classes and store operations. - /// The store operations have to be sorted ascendingly by sequence number. + /// The store operations have to be sorted by sequence number in ascending order. CSECodeGenerator(ExpressionClasses& _expressionClasses, StoreOperations const& _storeOperations); /// @returns the assembly items generated from the given requirements