From 965f005a989e0b7fb5a46f364bfb96dad2e18aae Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 24 Mar 2015 14:53:15 +0100 Subject: [PATCH 01/12] 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/12] 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/12] 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/12] 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/12] 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 df4f1258e849744920addadf3e22e256820d0857 Mon Sep 17 00:00:00 2001 From: Marek Kotewicz Date: Mon, 30 Mar 2015 14:02:27 +0200 Subject: [PATCH 06/12] failing tests for getting genesis block --- test/ClientBase.cpp | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/test/ClientBase.cpp b/test/ClientBase.cpp index 2197ac830..304182cfc 100644 --- a/test/ClientBase.cpp +++ b/test/ClientBase.cpp @@ -34,34 +34,45 @@ BOOST_AUTO_TEST_CASE(blocks) { enumerateClients([](Json::Value const& _json, dev::eth::ClientBase& _client) -> void { - for (string const& name: _json["postState"].getMemberNames()) + auto compareState = [&_client](Json::Value const& _o, string const& _name, BlockNumber _blockNumber) -> void { - Json::Value o = _json["postState"][name]; - Address address(name); + Address address(_name); // balanceAt - u256 expectedBalance = u256(o["balance"].asString()); - u256 balance = _client.balanceAt(address); + u256 expectedBalance = u256(_o["balance"].asString()); + u256 balance = _client.balanceAt(address, _blockNumber); ETH_CHECK_EQUAL(expectedBalance, balance); // countAt - u256 expectedCount = u256(o["nonce"].asString()); - u256 count = _client.countAt(address); + u256 expectedCount = u256(_o["nonce"].asString()); + u256 count = _client.countAt(address, _blockNumber); ETH_CHECK_EQUAL(expectedCount, count); // stateAt - for (string const& pos: o["storage"].getMemberNames()) + for (string const& pos: _o["storage"].getMemberNames()) { - u256 expectedState = u256(o["storage"][pos].asString()); - u256 state = _client.stateAt(address, u256(pos)); + u256 expectedState = u256(_o["storage"][pos].asString()); + u256 state = _client.stateAt(address, u256(pos), _blockNumber); ETH_CHECK_EQUAL(expectedState, state); } // codeAt - bytes expectedCode = fromHex(o["code"].asString()); - bytes code = _client.codeAt(address); + bytes expectedCode = fromHex(_o["code"].asString()); + bytes code = _client.codeAt(address, _blockNumber); ETH_CHECK_EQUAL_COLLECTIONS(expectedCode.begin(), expectedCode.end(), - code.begin(), code.end()); + code.begin(), code.end()); + }; + + for (string const& name: _json["postState"].getMemberNames()) + { + Json::Value o = _json["postState"][name]; + compareState(o, name, PendingBlock); + } + + for (string const& name: _json["pre"].getMemberNames()) + { + Json::Value o = _json["pre"][name]; + compareState(o, name, 0); } // number From a224fd2727f33b7b096e3aa60b185d2839455123 Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 31 Mar 2015 11:26:25 +0200 Subject: [PATCH 07/12] fix #1474 --- mix/qml/CodeEditorView.qml | 10 ++++++++++ mix/qml/WebPreview.qml | 7 ++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/mix/qml/CodeEditorView.qml b/mix/qml/CodeEditorView.qml index b100dffd0..7ea1e30d9 100644 --- a/mix/qml/CodeEditorView.qml +++ b/mix/qml/CodeEditorView.qml @@ -11,6 +11,16 @@ Item { signal breakpointsChanged(string documentId) signal isCleanChanged(var isClean, string documentId) + function getDocumentIdByName(fileName) + { + for (var i = 0; i < editorListModel.count; i++) { + if (editorListModel.get(i).fileName === fileName) { + return editorListModel.get(i).documentId; + } + } + return null; + } + function getDocumentText(documentId) { for (var i = 0; i < editorListModel.count; i++) { if (editorListModel.get(i).documentId === documentId) { diff --git a/mix/qml/WebPreview.qml b/mix/qml/WebPreview.qml index 8507c9b46..0bf31b326 100644 --- a/mix/qml/WebPreview.qml +++ b/mix/qml/WebPreview.qml @@ -155,18 +155,19 @@ Item { //document request if (urlPath === "/") urlPath = "/index.html"; - var documentId = urlPath.substr(urlPath.lastIndexOf("/") + 1); + var documentName = urlPath.substr(urlPath.lastIndexOf("/") + 1); + var documentId = projectModel.codeEditor.getDocumentIdByName(documentName); var content = ""; if (projectModel.codeEditor.isDocumentOpen(documentId)) content = projectModel.codeEditor.getDocumentText(documentId); else { var doc = projectModel.getDocument(documentId); - if (doc !== undefined) + if (doc) content = fileIo.readFile(doc.path); } - if (documentId === urlInput.text.replace(httpServer.url + "/", "")) { + if (documentName === urlInput.text.replace(httpServer.url + "/", "")) { //root page, inject deployment script content = "\n" + content; _request.setResponseContentType("text/html"); From 0e48d266c8cd2d94b2f1568665eeeaaf9fc88272 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 31 Mar 2015 11:37:10 +0200 Subject: [PATCH 08/12] Mix: fixed editor clipboard for os x --- mix/qml/html/codeeditor.js | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/mix/qml/html/codeeditor.js b/mix/qml/html/codeeditor.js index 299691ec0..4beae1955 100644 --- a/mix/qml/html/codeeditor.js +++ b/mix/qml/html/codeeditor.js @@ -22,17 +22,11 @@ editor.on("change", function(eMirror, object) { }); var mac = /Mac/.test(navigator.platform); +var extraKeys = {}; if (mac === true) { - editor.setOption("extraKeys", { - "Cmd-V": function(cm) { - cm.replaceSelection(clipboard); - }, - "Cmd-X": function(cm) { - window.document.execCommand("cut"); - }, - "Cmd-C": function(cm) { - window.document.execCommand("copy"); - }}); + extraKeys["Cmd-V"] = function(cm) { cm.replaceSelection(clipboard); }; + extraKeys["Cmd-X"] = function(cm) { window.document.execCommand("cut"); }; + extraKeys["Cmd-C"] = function(cm) { window.document.execCommand("copy"); }; } makeMarker = function() { @@ -102,16 +96,14 @@ setMode = function(mode) { if (mode === "javascript") { ternServer = new CodeMirror.TernServer({defs: [ ecma5Spec() ]}); - editor.setOption("extraKeys", { - "Ctrl-Space": function(cm) { ternServer.complete(cm); }, - "Ctrl-I": function(cm) { ternServer.showType(cm); }, - "Ctrl-O": function(cm) { ternServer.showDocs(cm); }, - "Alt-.": function(cm) { ternServer.jumpToDef(cm); }, - "Alt-,": function(cm) { ternServer.jumpBack(cm); }, - "Ctrl-Q": function(cm) { ternServer.rename(cm); }, - "Ctrl-.": function(cm) { ternServer.selectName(cm); }, - "'.'": function(cm) { setTimeout(function() { ternServer.complete(cm); }, 100); throw CodeMirror.Pass; } - }) + extraKeys["Ctrl-Space"] = function(cm) { ternServer.complete(cm); }; + extraKeys["Ctrl-I"] = function(cm) { ternServer.showType(cm); }; + extraKeys["Ctrl-O"] = function(cm) { ternServer.showDocs(cm); }; + extraKeys["Alt-."] = function(cm) { ternServer.jumpToDef(cm); }; + extraKeys["Alt-,"] = function(cm) { ternServer.jumpBack(cm); }; + extraKeys["Ctrl-Q"] = function(cm) { ternServer.rename(cm); }; + extraKeys["Ctrl-."] = function(cm) { ternServer.selectName(cm); }; + extraKeys["'.'"] = function(cm) { setTimeout(function() { ternServer.complete(cm); }, 100); throw CodeMirror.Pass; }; editor.on("cursorActivity", function(cm) { ternServer.updateArgHints(cm); }); } else if (mode === "solidity") @@ -119,9 +111,7 @@ setMode = function(mode) { CodeMirror.commands.autocomplete = function(cm) { CodeMirror.showHint(cm, CodeMirror.hint.anyword); } - editor.setOption("extraKeys", { - "Ctrl-Space": "autocomplete" - }) + extraKeys["Ctrl-Space"] = "autocomplete"; } }; @@ -199,3 +189,5 @@ compilationComplete = function() } compilationCompleteBool = true; } + +editor.setOption("extraKeys", extraKeys); From ab0501a70c4c96bc71becc9b20ac4e7a3d45791b Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 31 Mar 2015 12:03:26 +0200 Subject: [PATCH 09/12] apply extrakeys on mode change --- mix/qml/html/codeeditor.js | 1 + 1 file changed, 1 insertion(+) diff --git a/mix/qml/html/codeeditor.js b/mix/qml/html/codeeditor.js index 4beae1955..5d63a7a83 100644 --- a/mix/qml/html/codeeditor.js +++ b/mix/qml/html/codeeditor.js @@ -113,6 +113,7 @@ setMode = function(mode) { } extraKeys["Ctrl-Space"] = "autocomplete"; } + editor.setOption("extraKeys", extraKeys); }; setClipboardBase64 = function(text) { From 2ad558b9a962c04f856f304da6e6c7bd8ae8e134 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 Mar 2015 13:28:28 +0200 Subject: [PATCH 10/12] Started minor refactor - still API compatible though. --- libethash/CMakeLists.txt | 7 ++++ libethash/data_sizes.h | 4 +-- libethash/ethash.h | 75 ++++++++++++++++++++++------------------ libethash/internal.c | 14 ++++---- libethash/internal.h | 2 +- 5 files changed, 58 insertions(+), 44 deletions(-) diff --git a/libethash/CMakeLists.txt b/libethash/CMakeLists.txt index 38fc821c0..c92240086 100644 --- a/libethash/CMakeLists.txt +++ b/libethash/CMakeLists.txt @@ -12,6 +12,7 @@ endif() set(FILES util.c util.h + io.c internal.c ethash.h endian.h @@ -19,6 +20,12 @@ set(FILES util.c fnv.h data_sizes.h) +if (MSVC) + list(APPEND FILES io_win32.c) +else() + list(APPEND FILES io_posix.c) +endif() + if (NOT CRYPTOPP_FOUND) find_package(CryptoPP 5.6.2) endif() diff --git a/libethash/data_sizes.h b/libethash/data_sizes.h index 3b747b3ea..cf52ae4f8 100644 --- a/libethash/data_sizes.h +++ b/libethash/data_sizes.h @@ -48,7 +48,7 @@ extern "C" { // Sow[i*HashBytes]; j++]]]][[2]][[1]] -static const size_t dag_sizes[2048] = { +static const uint64_t dag_sizes[2048] = { 1073739904U, 1082130304U, 1090514816U, 1098906752U, 1107293056U, 1115684224U, 1124070016U, 1132461952U, 1140849536U, 1149232768U, 1157627776U, 1166013824U, 1174404736U, 1182786944U, 1191180416U, @@ -477,7 +477,7 @@ static const size_t dag_sizes[2048] = { // While[! PrimeQ[i], i--]; // Sow[i*HashBytes]; j++]]]][[2]][[1]] -const size_t cache_sizes[2048] = { +const uint64_t cache_sizes[2048] = { 16776896U, 16907456U, 17039296U, 17170112U, 17301056U, 17432512U, 17563072U, 17693888U, 17824192U, 17955904U, 18087488U, 18218176U, 18349504U, 18481088U, 18611392U, 18742336U, 18874304U, 19004224U, 19135936U, 19267264U, 19398208U, diff --git a/libethash/ethash.h b/libethash/ethash.h index a7159de65..eb3097307 100644 --- a/libethash/ethash.h +++ b/libethash/ethash.h @@ -43,26 +43,26 @@ extern "C" { #endif typedef struct ethash_params { - size_t full_size; // Size of full data set (in bytes, multiple of mix size (128)). - size_t cache_size; // Size of compute cache (in bytes, multiple of node size (64)). + uint64_t full_size; // Size of full data set (in bytes, multiple of mix size (128)). + uint64_t cache_size; // Size of compute cache (in bytes, multiple of node size (64)). } ethash_params; typedef struct ethash_return_value { - uint8_t result[32]; - uint8_t mix_hash[32]; + uint8_t result[32]; + uint8_t mix_hash[32]; } ethash_return_value; -size_t ethash_get_datasize(const uint32_t block_number); -size_t ethash_get_cachesize(const uint32_t block_number); +uint64_t ethash_get_datasize(const uint32_t block_number); +uint64_t ethash_get_cachesize(const uint32_t block_number); // initialize the parameters static inline void ethash_params_init(ethash_params *params, const uint32_t block_number) { - params->full_size = ethash_get_datasize(block_number); - params->cache_size = ethash_get_cachesize(block_number); + params->full_size = ethash_get_datasize(block_number); + params->cache_size = ethash_get_cachesize(block_number); } typedef struct ethash_cache { - void *mem; + void *mem; } ethash_cache; void ethash_mkcache(ethash_cache *cache, ethash_params const *params, const uint8_t seed[32]); @@ -72,44 +72,51 @@ void ethash_light(ethash_return_value *ret, ethash_cache const *cache, ethash_pa void ethash_get_seedhash(uint8_t seedhash[32], const uint32_t block_number); static inline void ethash_prep_light(void *cache, ethash_params const *params, const uint8_t seed[32]) { - ethash_cache c; - c.mem = cache; - ethash_mkcache(&c, params, seed); + ethash_cache c; + c.mem = cache; + ethash_mkcache(&c, params, seed); } static inline void ethash_compute_light(ethash_return_value *ret, void const *cache, ethash_params const *params, const uint8_t header_hash[32], const uint64_t nonce) { - ethash_cache c; - c.mem = (void *) cache; - ethash_light(ret, &c, params, header_hash, nonce); + ethash_cache c; + c.mem = (void *) cache; + ethash_light(ret, &c, params, header_hash, nonce); } static inline void ethash_prep_full(void *full, ethash_params const *params, void const *cache) { - ethash_cache c; - c.mem = (void *) cache; - ethash_compute_full_data(full, params, &c); + ethash_cache c; + c.mem = (void *) cache; + ethash_compute_full_data(full, params, &c); } static inline void ethash_compute_full(ethash_return_value *ret, void const *full, ethash_params const *params, const uint8_t header_hash[32], const uint64_t nonce) { - ethash_full(ret, full, params, header_hash, nonce); + ethash_full(ret, full, params, header_hash, nonce); } -// Returns if hash is less than or equal to difficulty -static inline int ethash_check_difficulty( - const uint8_t hash[32], - const uint8_t difficulty[32]) { - // Difficulty is big endian - for (int i = 0; i < 32; i++) { - if (hash[i] == difficulty[i]) continue; - return hash[i] < difficulty[i]; - } - return 1; +/// @brief Compare two s256-bit big-endian values. +/// @returns 1 if @a a is less than or equal to @a b, 0 otherwise. +/// Both parameters are 256-bit big-endian values. +static inline int ethash_leq_be256(const uint8_t a[32], const uint8_t b[32]) { + // Boundary is big endian + for (int i = 0; i < 32; i++) { + if (a[i] == b[i]) + continue; + return a[i] < b[i]; + } + return 1; } -int ethash_quick_check_difficulty( - const uint8_t header_hash[32], - const uint64_t nonce, - const uint8_t mix_hash[32], - const uint8_t difficulty[32]); +/// Perofrms a cursory check on the validity of the nonce. +/// @returns 1 if the nonce may possibly be valid for the given header_hash & boundary. +/// @p boundary equivalent to 2 ^ 256 / block_difficulty, represented as a 256-bit big-endian. +int ethash_preliminary_check_boundary( + const uint8_t header_hash[32], + const uint64_t nonce, + const uint8_t mix_hash[32], + const uint8_t boundary[32]); + +#define ethash_quick_check_difficulty ethash_preliminary_check_boundary +#define ethash_check_difficulty ethash_leq_be256 #ifdef __cplusplus } diff --git a/libethash/internal.c b/libethash/internal.c index 0a7e767e7..ae9b95065 100644 --- a/libethash/internal.c +++ b/libethash/internal.c @@ -37,12 +37,12 @@ #include "sha3.h" #endif // WITH_CRYPTOPP -size_t ethash_get_datasize(const uint32_t block_number) { +uint64_t ethash_get_datasize(const uint32_t block_number) { assert(block_number / EPOCH_LENGTH < 2048); return dag_sizes[block_number / EPOCH_LENGTH]; } -size_t ethash_get_cachesize(const uint32_t block_number) { +uint64_t ethash_get_cachesize(const uint32_t block_number) { assert(block_number / EPOCH_LENGTH < 2048); return cache_sizes[block_number / EPOCH_LENGTH]; } @@ -280,15 +280,15 @@ void ethash_get_seedhash(uint8_t seedhash[32], const uint32_t block_number) { SHA3_256(seedhash, seedhash, 32); } -int ethash_quick_check_difficulty( +int ethash_preliminary_check_boundary( const uint8_t header_hash[32], const uint64_t nonce, const uint8_t mix_hash[32], - const uint8_t difficulty[32]) { + const uint8_t difficulty[32]) { - uint8_t return_hash[32]; + uint8_t return_hash[32]; ethash_quick_hash(return_hash, header_hash, nonce, mix_hash); - return ethash_check_difficulty(return_hash, difficulty); + return ethash_leq_be256(return_hash, difficulty); } void ethash_full(ethash_return_value *ret, void const *full_mem, ethash_params const *params, const uint8_t previous_hash[32], const uint64_t nonce) { @@ -297,4 +297,4 @@ void ethash_full(ethash_return_value *ret, void const *full_mem, ethash_params c void ethash_light(ethash_return_value *ret, ethash_cache const *cache, ethash_params const *params, const uint8_t previous_hash[32], const uint64_t nonce) { ethash_hash(ret, NULL, cache, params, previous_hash, nonce); -} \ No newline at end of file +} diff --git a/libethash/internal.h b/libethash/internal.h index bcbacdaa4..ddd06e8f4 100644 --- a/libethash/internal.h +++ b/libethash/internal.h @@ -3,7 +3,7 @@ #include "endian.h" #include "ethash.h" -#define ENABLE_SSE 1 +#define ENABLE_SSE 0 #if defined(_M_X64) && ENABLE_SSE #include From 5039e4a9eeffd522d06fedc80ae339c84b39d78e Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 31 Mar 2015 14:02:08 +0200 Subject: [PATCH 11/12] Add missing files. --- libethash/io.c | 89 +++++++++++++++++++++++++++++++++ libethash/io.h | 116 +++++++++++++++++++++++++++++++++++++++++++ libethash/io_posix.c | 76 ++++++++++++++++++++++++++++ libethash/io_win32.c | 73 +++++++++++++++++++++++++++ 4 files changed, 354 insertions(+) create mode 100644 libethash/io.c create mode 100644 libethash/io.h create mode 100644 libethash/io_posix.c create mode 100644 libethash/io_win32.c diff --git a/libethash/io.c b/libethash/io.c new file mode 100644 index 000000000..dd4f1f9e8 --- /dev/null +++ b/libethash/io.c @@ -0,0 +1,89 @@ +/* + This file is part of ethash. + + ethash is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ethash is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with ethash. If not, see . +*/ +/** @file io.c + * @author Lefteris Karapetsas + * @date 2015 + */ +#include "io.h" +#include +#include + +// silly macro to save some typing +#define PASS_ARR(c_) (c_), sizeof(c_) + +static bool ethash_io_write_file(char const *dirname, + char const* filename, + size_t filename_length, + void const* data, + size_t data_size) +{ + bool ret = false; + char *fullname = ethash_io_create_filename(dirname, filename, filename_length); + if (!fullname) { + return false; + } + FILE *f = fopen(fullname, "wb"); + if (!f) { + goto free_name; + } + if (data_size != fwrite(data, 1, data_size, f)) { + goto close; + } + + ret = true; +close: + fclose(f); +free_name: + free(fullname); + return ret; +} + +bool ethash_io_write(char const *dirname, + ethash_params const* params, + ethash_blockhash_t seedhash, + void const* cache, + uint8_t **data, + size_t *data_size) +{ + char info_buffer[DAG_MEMO_BYTESIZE]; + // allocate the bytes + uint8_t *temp_data_ptr = malloc(params->full_size); + if (!temp_data_ptr) { + goto end; + } + ethash_compute_full_data(temp_data_ptr, params, cache); + + if (!ethash_io_write_file(dirname, PASS_ARR(DAG_FILE_NAME), temp_data_ptr, params->full_size)) { + goto fail_free; + } + + ethash_io_serialize_info(REVISION, seedhash, info_buffer); + if (!ethash_io_write_file(dirname, PASS_ARR(DAG_MEMO_NAME), info_buffer, DAG_MEMO_BYTESIZE)) { + goto fail_free; + } + + *data = temp_data_ptr; + *data_size = params->full_size; + return true; + +fail_free: + free(temp_data_ptr); +end: + return false; +} + +#undef PASS_ARR diff --git a/libethash/io.h b/libethash/io.h new file mode 100644 index 000000000..0fa292362 --- /dev/null +++ b/libethash/io.h @@ -0,0 +1,116 @@ +/* + This file is part of ethash. + + ethash is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ethash is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with ethash. If not, see . +*/ +/** @file io.h + * @author Lefteris Karapetsas + * @date 2015 + */ +#pragma once +#include +#include +#include +#include "ethash.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct ethash_blockhash { uint8_t b[32]; } ethash_blockhash_t; + +static const char DAG_FILE_NAME[] = "full"; +static const char DAG_MEMO_NAME[] = "full.info"; +// MSVC thinks that "static const unsigned int" is not a compile time variable. Sorry for the #define :( +#define DAG_MEMO_BYTESIZE 36 + +/// Possible return values of @see ethash_io_prepare +enum ethash_io_rc { + ETHASH_IO_FAIL = 0, ///< There has been an IO failure + ETHASH_IO_MEMO_MISMATCH, ///< Memo file either did not exist or there was content mismatch + ETHASH_IO_MEMO_MATCH, ///< Memo file existed and contents matched. No need to do anything +}; + +/** + * Prepares io for ethash + * + * Create the DAG directory if it does not exist, and check if the memo file matches. + * If it does not match then it's deleted to pave the way for @ref ethash_io_write() + * + * @param dirname A null terminated c-string of the path of the ethash + * data directory. If it does not exist it's created. + * @param seedhash The seedhash of the current block number + * @return For possible return values @see enum ethash_io_rc + */ +enum ethash_io_rc ethash_io_prepare(char const *dirname, ethash_blockhash_t seedhash); +/** + * Fully computes data and writes it to the file on disk. + * + * This function should be called after @see ethash_io_prepare() and only if + * its return value is @c ETHASH_IO_MEMO_MISMATCH. Will write both the full data + * and the memo file. + * + * @param[in] dirname A null terminated c-string of the path of the ethash + * data directory. Has to exist. + * @param[in] params An ethash_params object containing the full size + * and the cache size + * @param[in] seedhash The seedhash of the current block number + * @param[in] cache The cache data. Would have usually been calulated by + * @see ethash_prep_light(). + * @param[out] data Pass a pointer to uint8_t by reference here. If the + * function is succesfull then this point to the allocated + * data calculated by @see ethash_prep_full(). Memory + * ownership is transfered to the callee. Remember that + * you eventually need to free this with a call to free(). + * @param[out] data_size Pass a size_t by value. If the function is succesfull + * then this will contain the number of bytes allocated + * for @a data. + * @return True for success and false in case of failure. + */ +bool ethash_io_write(char const *dirname, + ethash_params const* params, + ethash_blockhash_t seedhash, + void const* cache, + uint8_t **data, + size_t *data_size); + +static inline void ethash_io_serialize_info(uint32_t revision, + ethash_blockhash_t seed_hash, + char *output) +{ + // if .info is only consumed locally we don't really care about endianess + memcpy(output, &revision, 4); + memcpy(output + 4, &seed_hash, 32); +} + +static inline char *ethash_io_create_filename(char const *dirname, + char const* filename, + size_t filename_length) +{ + // in C the cast is not needed, but a C++ compiler will complain for invalid conversion + char *name = (char*)malloc(strlen(dirname) + filename_length); + if (!name) { + return NULL; + } + + name[0] = '\0'; + strcat(name, dirname); + strcat(name, filename); + return name; +} + + +#ifdef __cplusplus +} +#endif diff --git a/libethash/io_posix.c b/libethash/io_posix.c new file mode 100644 index 000000000..693bdf750 --- /dev/null +++ b/libethash/io_posix.c @@ -0,0 +1,76 @@ +/* + This file is part of ethash. + + ethash is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ethash is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with ethash. If not, see . +*/ +/** @file io_posix.c + * @author Lefteris Karapetsas + * @date 2015 + */ + +#include "io.h" +#include +#include +#include +#include +#include +#include + +enum ethash_io_rc ethash_io_prepare(char const *dirname, ethash_blockhash_t seedhash) +{ + char read_buffer[DAG_MEMO_BYTESIZE]; + char expect_buffer[DAG_MEMO_BYTESIZE]; + enum ethash_io_rc ret = ETHASH_IO_FAIL; + + // assert directory exists, full owner permissions and read/search for others + int rc = mkdir(dirname, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); + if (rc == -1 && errno != EEXIST) { + goto end; + } + + char *memofile = ethash_io_create_filename(dirname, DAG_MEMO_NAME, sizeof(DAG_MEMO_NAME)); + if (!memofile) { + goto end; + } + + // try to open memo file + FILE *f = fopen(memofile, "rb"); + if (!f) { + // file does not exist, so no checking happens. All is fine. + ret = ETHASH_IO_MEMO_MISMATCH; + goto free_memo; + } + + if (fread(read_buffer, 1, DAG_MEMO_BYTESIZE, f) != DAG_MEMO_BYTESIZE) { + goto close; + } + + ethash_io_serialize_info(REVISION, seedhash, expect_buffer); + if (memcmp(read_buffer, expect_buffer, DAG_MEMO_BYTESIZE) != 0) { + // we have different memo contents so delete the memo file + if (unlink(memofile) != 0) { + goto close; + } + ret = ETHASH_IO_MEMO_MISMATCH; + } + + ret = ETHASH_IO_MEMO_MATCH; + +close: + fclose(f); +free_memo: + free(memofile); +end: + return ret; +} diff --git a/libethash/io_win32.c b/libethash/io_win32.c new file mode 100644 index 000000000..2cabc939a --- /dev/null +++ b/libethash/io_win32.c @@ -0,0 +1,73 @@ +/* + This file is part of ethash. + + ethash is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ethash is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with ethash. If not, see . +*/ +/** @file io_win32.c + * @author Lefteris Karapetsas + * @date 2015 + */ + +#include "io.h" +#include +#include +#include + +enum ethash_io_rc ethash_io_prepare(char const *dirname, ethash_blockhash_t seedhash) +{ + char read_buffer[DAG_MEMO_BYTESIZE]; + char expect_buffer[DAG_MEMO_BYTESIZE]; + enum ethash_io_rc ret = ETHASH_IO_FAIL; + + // assert directory exists + int rc = _mkdir(dirname); + if (rc == -1 && errno != EEXIST) { + goto end; + } + + char *memofile = ethash_io_create_filename(dirname, DAG_MEMO_NAME, sizeof(DAG_MEMO_NAME)); + if (!memofile) { + goto end; + } + + // try to open memo file + FILE *f = fopen(memofile, "rb"); + if (!f) { + // file does not exist, so no checking happens. All is fine. + ret = ETHASH_IO_MEMO_MISMATCH; + goto free_memo; + } + + if (fread(read_buffer, 1, DAG_MEMO_BYTESIZE, f) != DAG_MEMO_BYTESIZE) { + goto close; + } + + ethash_io_serialize_info(REVISION, seedhash, expect_buffer); + if (memcmp(read_buffer, expect_buffer, DAG_MEMO_BYTESIZE) != 0) { + // we have different memo contents so delete the memo file + if (_unlink(memofile) != 0) { + goto close; + } + ret = ETHASH_IO_MEMO_MISMATCH; + } + + ret = ETHASH_IO_MEMO_MATCH; + +close: + fclose(f); +free_memo: + free(memofile); +end: + return ret; +} From 01e6801cf5e6b0f9f4cb6f50893e5354caf4137e Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 31 Mar 2015 15:10:21 +0200 Subject: [PATCH 12/12] 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