From afe887adc1523e215cbc6479fbada32410f27b55 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 27 May 2019 13:11:00 +0200 Subject: [PATCH 1/4] Split MovableChecker and introduce SideEffectsUpToMSize. --- libevmasm/SemanticInformation.cpp | 21 +++---- libevmasm/SemanticInformation.h | 6 ++ libyul/Dialect.h | 5 ++ libyul/backends/evm/EVMDialect.cpp | 11 +++- libyul/backends/wasm/WasmDialect.cpp | 2 + libyul/optimiser/Semantics.cpp | 84 ++++++++++++++++++---------- libyul/optimiser/Semantics.h | 55 +++++++++++++----- 7 files changed, 124 insertions(+), 60 deletions(-) diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index e6d5fc350..6b6b4611d 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -213,20 +213,15 @@ bool SemanticInformation::sideEffectFree(Instruction _instruction) // These are not really functional. assertThrow(!isDupInstruction(_instruction) && !isSwapInstruction(_instruction), AssemblyException, ""); - InstructionInfo info = instructionInfo(_instruction); - switch (_instruction) - { - // All the instructions that merely read memory are fine - // even though they are marked "sideEffects" in Instructions.cpp - case Instruction::KECCAK256: - case Instruction::MLOAD: + return !instructionInfo(_instruction).sideEffects; +} + +bool SemanticInformation::sideEffectFreeIfNoMSize(Instruction _instruction) +{ + if (_instruction == Instruction::KECCAK256 || _instruction == Instruction::MLOAD) return true; - default: - break; - } - if (info.sideEffects) - return false; - return true; + else + return sideEffectFree(_instruction); } bool SemanticInformation::invalidatesMemory(Instruction _instruction) diff --git a/libevmasm/SemanticInformation.h b/libevmasm/SemanticInformation.h index 9b256b58d..5f2d3b056 100644 --- a/libevmasm/SemanticInformation.h +++ b/libevmasm/SemanticInformation.h @@ -60,6 +60,12 @@ struct SemanticInformation /// This does not mean that it has to be deterministic or retrieve information from /// somewhere else than purely the values of its arguments. static bool sideEffectFree(Instruction _instruction); + /// @returns true if the instruction can be removed without changing the semantics. + /// This does not mean that it has to be deterministic or retrieve information from + /// somewhere else than purely the values of its arguments. + /// If true, the instruction is still allowed to influence the value returned by the + /// msize instruction. + static bool sideEffectFreeIfNoMSize(Instruction _instruction); /// @returns true if the given instruction modifies memory. static bool invalidatesMemory(Instruction _instruction); /// @returns true if the given instruction modifies storage (even indirectly). diff --git a/libyul/Dialect.h b/libyul/Dialect.h index 462187309..4fc33edbc 100644 --- a/libyul/Dialect.h +++ b/libyul/Dialect.h @@ -51,6 +51,11 @@ struct BuiltinFunction bool movable = false; /// If true, a call to this function can be omitted without changing semantics. bool sideEffectFree = false; + /// If true, a call to this function can be omitted without changing semantics if the + /// program does not contain the msize instruction. + bool sideEffectFreeIfNoMSize = false; + /// If true, this is the msize instruction. + bool isMSize = false; /// If true, can only accept literals as arguments and they cannot be moved to variables. bool literalArguments = false; }; diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index 36244d576..388383324 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -52,6 +52,8 @@ pair createEVMFunction( f.returns.resize(info.ret); f.movable = eth::SemanticInformation::movable(_instruction); f.sideEffectFree = eth::SemanticInformation::sideEffectFree(_instruction); + f.sideEffectFreeIfNoMSize = eth::SemanticInformation::sideEffectFreeIfNoMSize(_instruction); + f.isMSize = _instruction == dev::eth::Instruction::MSIZE; f.literalArguments = false; f.instruction = _instruction; f.generateCode = [_instruction]( @@ -73,6 +75,7 @@ pair createFunction( size_t _returns, bool _movable, bool _sideEffectFree, + bool _sideEffectFreeIfNoMSize, bool _literalArguments, std::function)> _generateCode ) @@ -85,6 +88,8 @@ pair createFunction( f.movable = _movable; f.literalArguments = _literalArguments; f.sideEffectFree = _sideEffectFree; + f.sideEffectFreeIfNoMSize = _sideEffectFreeIfNoMSize; + f.isMSize = false; f.instruction = {}; f.generateCode = std::move(_generateCode); return {name, f}; @@ -105,7 +110,7 @@ map createBuiltins(langutil::EVMVersion _evmVe if (_objectAccess) { - builtins.emplace(createFunction("datasize", 1, 1, true, true, true, []( + builtins.emplace(createFunction("datasize", 1, 1, true, true, true, true, []( FunctionCall const& _call, AbstractAssembly& _assembly, BuiltinContext& _context, @@ -126,7 +131,7 @@ map createBuiltins(langutil::EVMVersion _evmVe _assembly.appendDataSize(_context.subIDs.at(dataName)); } })); - builtins.emplace(createFunction("dataoffset", 1, 1, true, true, true, []( + builtins.emplace(createFunction("dataoffset", 1, 1, true, true, true, true, []( FunctionCall const& _call, AbstractAssembly& _assembly, BuiltinContext& _context, @@ -147,7 +152,7 @@ map createBuiltins(langutil::EVMVersion _evmVe _assembly.appendDataOffset(_context.subIDs.at(dataName)); } })); - builtins.emplace(createFunction("datacopy", 3, 0, false, false, false, []( + builtins.emplace(createFunction("datacopy", 3, 0, false, false, false, false, []( FunctionCall const&, AbstractAssembly& _assembly, BuiltinContext&, diff --git a/libyul/backends/wasm/WasmDialect.cpp b/libyul/backends/wasm/WasmDialect.cpp index 8b6ff0e73..b41b95d57 100644 --- a/libyul/backends/wasm/WasmDialect.cpp +++ b/libyul/backends/wasm/WasmDialect.cpp @@ -72,5 +72,7 @@ void WasmDialect::addFunction(string _name, size_t _params, size_t _returns) f.returns.resize(_returns); f.movable = false; f.sideEffectFree = false; + f.sideEffectFreeIfNoMSize = false; + f.isMSize = false; f.literalArguments = false; } diff --git a/libyul/optimiser/Semantics.cpp b/libyul/optimiser/Semantics.cpp index 6291ce688..f9e9e8338 100644 --- a/libyul/optimiser/Semantics.cpp +++ b/libyul/optimiser/Semantics.cpp @@ -33,9 +33,59 @@ using namespace std; using namespace dev; using namespace yul; -MovableChecker::MovableChecker(Dialect const& _dialect): - m_dialect(_dialect) +SideEffectsCollector::SideEffectsCollector(Dialect const& _dialect, Expression const& _expression): + SideEffectsCollector(_dialect) { + visit(_expression); +} + +SideEffectsCollector::SideEffectsCollector(Dialect const& _dialect, Statement const& _statement): + SideEffectsCollector(_dialect) +{ + visit(_statement); +} + +SideEffectsCollector::SideEffectsCollector(Dialect const& _dialect, Block const& _ast): + SideEffectsCollector(_dialect) +{ + operator()(_ast); +} + +void SideEffectsCollector::operator()(FunctionalInstruction const& _instr) +{ + ASTWalker::operator()(_instr); + + if (!eth::SemanticInformation::movable(_instr.instruction)) + m_movable = false; + if (!eth::SemanticInformation::sideEffectFree(_instr.instruction)) + m_sideEffectFree = false; + if (!eth::SemanticInformation::sideEffectFreeIfNoMSize(_instr.instruction)) + m_sideEffectFreeIfNoMSize = false; + if (_instr.instruction == eth::Instruction::MSIZE) + m_containsMSize = true; +} + +void SideEffectsCollector::operator()(FunctionCall const& _functionCall) +{ + ASTWalker::operator()(_functionCall); + + if (BuiltinFunction const* f = m_dialect.builtin(_functionCall.functionName.name)) + { + if (!f->movable) + m_movable = false; + if (!f->sideEffectFree) + m_sideEffectFree = false; + if (!f->sideEffectFreeIfNoMSize) + m_sideEffectFreeIfNoMSize = false; + if (f->isMSize) + m_containsMSize = true; + } + else + { + m_movable = false; + m_sideEffectFree = false; + m_sideEffectFreeIfNoMSize = false; + } } MovableChecker::MovableChecker(Dialect const& _dialect, Expression const& _expression): @@ -46,38 +96,10 @@ MovableChecker::MovableChecker(Dialect const& _dialect, Expression const& _expre void MovableChecker::operator()(Identifier const& _identifier) { - ASTWalker::operator()(_identifier); + SideEffectsCollector::operator()(_identifier); m_variableReferences.emplace(_identifier.name); } -void MovableChecker::operator()(FunctionalInstruction const& _instr) -{ - ASTWalker::operator()(_instr); - - if (!eth::SemanticInformation::movable(_instr.instruction)) - m_movable = false; - if (!eth::SemanticInformation::sideEffectFree(_instr.instruction)) - m_sideEffectFree = false; -} - -void MovableChecker::operator()(FunctionCall const& _functionCall) -{ - ASTWalker::operator()(_functionCall); - - if (BuiltinFunction const* f = m_dialect.builtin(_functionCall.functionName.name)) - { - if (!f->movable) - m_movable = false; - if (!f->sideEffectFree) - m_sideEffectFree = false; - } - else - { - m_movable = false; - m_sideEffectFree = false; - } -} - void MovableChecker::visit(Statement const&) { assertThrow(false, OptimizerException, "Movability for statement requested."); diff --git a/libyul/optimiser/Semantics.h b/libyul/optimiser/Semantics.h index b50169f97..3d9966756 100644 --- a/libyul/optimiser/Semantics.h +++ b/libyul/optimiser/Semantics.h @@ -29,38 +29,67 @@ namespace yul struct Dialect; /** - * Specific AST walker that determines whether an expression is movable. + * Specific AST walker that determines side-effect free-ness and movability of code. + * Enters into function definitions. */ -class MovableChecker: public ASTWalker +class SideEffectsCollector: public ASTWalker { public: - explicit MovableChecker(Dialect const& _dialect); - MovableChecker(Dialect const& _dialect, Expression const& _expression); + explicit SideEffectsCollector(Dialect const& _dialect): m_dialect(_dialect) {} + SideEffectsCollector(Dialect const& _dialect, Expression const& _expression); + SideEffectsCollector(Dialect const& _dialect, Statement const& _statement); + SideEffectsCollector(Dialect const& _dialect, Block const& _ast); - void operator()(Identifier const& _identifier) override; + using ASTWalker::operator(); void operator()(FunctionalInstruction const& _functionalInstruction) override; void operator()(FunctionCall const& _functionCall) override; - /// Disallow visiting anything apart from Expressions (this throws). - void visit(Statement const&) override; - using ASTWalker::visit; - bool movable() const { return m_movable; } bool sideEffectFree() const { return m_sideEffectFree; } - - std::set const& referencedVariables() const { return m_variableReferences; } + bool sideEffectFreeIfNoMSize() const { return m_sideEffectFreeIfNoMSize; } + bool containsMSize() const { return m_containsMSize; } private: Dialect const& m_dialect; - /// Which variables the current expression references. - std::set m_variableReferences; /// Is the current expression movable or not. bool m_movable = true; /// Is the current expression side-effect free, i.e. can be removed /// without changing the semantics. bool m_sideEffectFree = true; + /// Is the current expression side-effect free up to msize, i.e. can be removed + /// without changing the semantics except for the value returned by the msize instruction. + bool m_sideEffectFreeIfNoMSize = true; + /// Does the current code contain the MSize operation? + /// Note that this is a purely syntactic property meaning that even if this is false, + /// the code can still contain calls to functions that contain the msize instruction. + bool m_containsMSize = false; }; +/** + * Specific AST walker that determines whether an expression is movable + * and collects the referenced variables. + * Can only be used on expressions. + */ +class MovableChecker: public SideEffectsCollector +{ +public: + explicit MovableChecker(Dialect const& _dialect): SideEffectsCollector(_dialect) {} + MovableChecker(Dialect const& _dialect, Expression const& _expression); + + void operator()(Identifier const& _identifier) override; + + /// Disallow visiting anything apart from Expressions (this throws). + void visit(Statement const&) override; + using ASTWalker::visit; + + std::set const& referencedVariables() const { return m_variableReferences; } + +private: + /// Which variables the current expression references. + std::set m_variableReferences; +}; + + /** * Helper class to find "irregular" control flow. * This includes termination, break and continue. From d7b5ea6761c35865ac854a3500d4709613af9c4c Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 27 May 2019 13:42:50 +0200 Subject: [PATCH 2/4] Allow msize optimization only if it is not present. --- libyul/optimiser/ExpressionInliner.cpp | 2 +- libyul/optimiser/ExpressionSimplifier.cpp | 2 +- .../optimiser/RedundantAssignEliminator.cpp | 2 +- libyul/optimiser/Semantics.h | 8 +++- libyul/optimiser/SimplificationRules.cpp | 2 +- libyul/optimiser/StackCompressor.cpp | 24 +++++++++-- libyul/optimiser/UnusedPruner.cpp | 42 +++++++++++++++---- libyul/optimiser/UnusedPruner.h | 14 +++++++ .../yulOptimizerTests/unusedPruner/msize.yul | 12 ++++++ .../unusedPruner/no_msize.yul | 9 ++++ 10 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 test/libyul/yulOptimizerTests/unusedPruner/msize.yul create mode 100644 test/libyul/yulOptimizerTests/unusedPruner/no_msize.yul diff --git a/libyul/optimiser/ExpressionInliner.cpp b/libyul/optimiser/ExpressionInliner.cpp index 43a6a3342..991520ef8 100644 --- a/libyul/optimiser/ExpressionInliner.cpp +++ b/libyul/optimiser/ExpressionInliner.cpp @@ -55,7 +55,7 @@ void ExpressionInliner::visit(Expression& _expression) bool movable = boost::algorithm::all_of( funCall.arguments, - [=](Expression const& _arg) { return MovableChecker(m_dialect, _arg).movable(); } + [=](Expression const& _arg) { return SideEffectsCollector(m_dialect, _arg).movable(); } ); if (m_inlinableFunctions.count(funCall.functionName.name) && movable) { diff --git a/libyul/optimiser/ExpressionSimplifier.cpp b/libyul/optimiser/ExpressionSimplifier.cpp index c1764c792..defe3598a 100644 --- a/libyul/optimiser/ExpressionSimplifier.cpp +++ b/libyul/optimiser/ExpressionSimplifier.cpp @@ -43,7 +43,7 @@ void ExpressionSimplifier::visit(Expression& _expression) // so if the value of the variable is not movable, the expression that references // the variable still is. - if (match->removesNonConstants && !MovableChecker(m_dialect, _expression).movable()) + if (match->removesNonConstants && !SideEffectsCollector(m_dialect, _expression).movable()) return; _expression = match->action().toExpression(locationOf(_expression)); } diff --git a/libyul/optimiser/RedundantAssignEliminator.cpp b/libyul/optimiser/RedundantAssignEliminator.cpp index 60e2dab4d..8baa4e7dc 100644 --- a/libyul/optimiser/RedundantAssignEliminator.cpp +++ b/libyul/optimiser/RedundantAssignEliminator.cpp @@ -284,7 +284,7 @@ void RedundantAssignEliminator::finalize( { State const state = assignment.second == State::Undecided ? _finalState : assignment.second; - if (state == State::Unused && MovableChecker{*m_dialect, *assignment.first->value}.movable()) + if (state == State::Unused && SideEffectsCollector{*m_dialect, *assignment.first->value}.movable()) // TODO the only point where we actually need this // to be a set is for the for loop m_pendingRemovals.insert(assignment.first); diff --git a/libyul/optimiser/Semantics.h b/libyul/optimiser/Semantics.h index 3d9966756..6c36ca0d4 100644 --- a/libyul/optimiser/Semantics.h +++ b/libyul/optimiser/Semantics.h @@ -45,7 +45,13 @@ public: void operator()(FunctionCall const& _functionCall) override; bool movable() const { return m_movable; } - bool sideEffectFree() const { return m_sideEffectFree; } + bool sideEffectFree(bool _allowMSizeModification = false) const + { + if (_allowMSizeModification) + return sideEffectFreeIfNoMSize(); + else + return m_sideEffectFree; + } bool sideEffectFreeIfNoMSize() const { return m_sideEffectFreeIfNoMSize; } bool containsMSize() const { return m_containsMSize; } diff --git a/libyul/optimiser/SimplificationRules.cpp b/libyul/optimiser/SimplificationRules.cpp index 861f8b25b..48e883701 100644 --- a/libyul/optimiser/SimplificationRules.cpp +++ b/libyul/optimiser/SimplificationRules.cpp @@ -187,7 +187,7 @@ bool Pattern::matches( assertThrow(firstMatch, OptimizerException, "Match set but to null."); return SyntacticallyEqual{}(*firstMatch, _expr) && - MovableChecker(_dialect, _expr).movable(); + SideEffectsCollector(_dialect, _expr).movable(); } else if (m_kind == PatternKind::Any) (*m_matchGroups)[m_matchGroup] = &_expr; diff --git a/libyul/optimiser/StackCompressor.cpp b/libyul/optimiser/StackCompressor.cpp index 6d35cee04..826bd2e05 100644 --- a/libyul/optimiser/StackCompressor.cpp +++ b/libyul/optimiser/StackCompressor.cpp @@ -113,7 +113,12 @@ public: }; template -void eliminateVariables(Dialect const& _dialect, ASTNode& _node, size_t _numVariables) +void eliminateVariables( + Dialect const& _dialect, + ASTNode& _node, + size_t _numVariables, + bool _allowMSizeOptimization +) { RematCandidateSelector selector{_dialect}; selector(_node); @@ -143,7 +148,7 @@ void eliminateVariables(Dialect const& _dialect, ASTNode& _node, size_t _numVari } Rematerialiser::run(_dialect, _node, std::move(varsToEliminate)); - UnusedPruner::runUntilStabilised(_dialect, _node); + UnusedPruner::runUntilStabilised(_dialect, _node, _allowMSizeOptimization); } } @@ -159,6 +164,7 @@ bool StackCompressor::run( _ast.statements.size() > 0 && _ast.statements.at(0).type() == typeid(Block), "Need to run the function grouper before the stack compressor." ); + bool allowMSizeOptimzation = !SideEffectsCollector(_dialect, _ast).containsMSize(); for (size_t iterations = 0; iterations < _maxIterations; iterations++) { map stackSurplus = CompilabilityChecker::run(_dialect, _ast, _optimizeStackAllocation); @@ -168,7 +174,12 @@ bool StackCompressor::run( if (stackSurplus.count(YulString{})) { yulAssert(stackSurplus.at({}) > 0, "Invalid surplus value."); - eliminateVariables(_dialect, boost::get(_ast.statements.at(0)), stackSurplus.at({})); + eliminateVariables( + _dialect, + boost::get(_ast.statements.at(0)), + stackSurplus.at({}), + allowMSizeOptimzation + ); } for (size_t i = 1; i < _ast.statements.size(); ++i) @@ -178,7 +189,12 @@ bool StackCompressor::run( continue; yulAssert(stackSurplus.at(fun.name) > 0, "Invalid surplus value."); - eliminateVariables(_dialect, fun, stackSurplus.at(fun.name)); + eliminateVariables( + _dialect, + fun, + stackSurplus.at(fun.name), + allowMSizeOptimzation + ); } } return false; diff --git a/libyul/optimiser/UnusedPruner.cpp b/libyul/optimiser/UnusedPruner.cpp index 858c5223b..00ccbc8e5 100644 --- a/libyul/optimiser/UnusedPruner.cpp +++ b/libyul/optimiser/UnusedPruner.cpp @@ -32,16 +32,28 @@ using namespace std; using namespace dev; using namespace yul; -UnusedPruner::UnusedPruner(Dialect const& _dialect, Block& _ast, set const& _externallyUsedFunctions): - m_dialect(_dialect) +UnusedPruner::UnusedPruner( + Dialect const& _dialect, + Block& _ast, + bool _allowMSizeOptimization, + set const& _externallyUsedFunctions +): + m_dialect(_dialect), + m_allowMSizeOptimization(_allowMSizeOptimization) { m_references = ReferencesCounter::countReferences(_ast); for (auto const& f: _externallyUsedFunctions) ++m_references[f]; } -UnusedPruner::UnusedPruner(Dialect const& _dialect, FunctionDefinition& _function, set const& _externallyUsedFunctions): - m_dialect(_dialect) +UnusedPruner::UnusedPruner( + Dialect const& _dialect, + FunctionDefinition& _function, + bool _allowMSizeOptimization, + set const& _externallyUsedFunctions +): + m_dialect(_dialect), + m_allowMSizeOptimization(_allowMSizeOptimization) { m_references = ReferencesCounter::countReferences(_function); for (auto const& f: _externallyUsedFunctions) @@ -75,7 +87,7 @@ void UnusedPruner::operator()(Block& _block) { if (!varDecl.value) statement = Block{std::move(varDecl.location), {}}; - else if (MovableChecker(m_dialect, *varDecl.value).sideEffectFree()) + else if (SideEffectsCollector(m_dialect, *varDecl.value).sideEffectFree(m_allowMSizeOptimization)) { subtractReferences(ReferencesCounter::countReferences(*varDecl.value)); statement = Block{std::move(varDecl.location), {}}; @@ -93,7 +105,7 @@ void UnusedPruner::operator()(Block& _block) else if (statement.type() == typeid(ExpressionStatement)) { ExpressionStatement& exprStmt = boost::get(statement); - if (MovableChecker(m_dialect, exprStmt.expression).sideEffectFree()) + if (SideEffectsCollector(m_dialect, exprStmt.expression).sideEffectFree(m_allowMSizeOptimization)) { subtractReferences(ReferencesCounter::countReferences(exprStmt.expression)); statement = Block{std::move(exprStmt.location), {}}; @@ -108,27 +120,41 @@ void UnusedPruner::operator()(Block& _block) void UnusedPruner::runUntilStabilised( Dialect const& _dialect, Block& _ast, + bool _allowMSizeOptization, set const& _externallyUsedFunctions ) { + _allowMSizeOptization = !SideEffectsCollector(_dialect, _ast).containsMSize(); + while (true) { - UnusedPruner pruner(_dialect, _ast, _externallyUsedFunctions); + UnusedPruner pruner(_dialect, _ast, _allowMSizeOptization, _externallyUsedFunctions); pruner(_ast); if (!pruner.shouldRunAgain()) return; } } +void UnusedPruner::runUntilStabilised( + Dialect const& _dialect, + Block& _ast, + set const& _externallyUsedFunctions +) +{ + bool allowMSizeOptimization = !SideEffectsCollector(_dialect, _ast).containsMSize(); + runUntilStabilised(_dialect, _ast, allowMSizeOptimization, _externallyUsedFunctions); +} + void UnusedPruner::runUntilStabilised( Dialect const& _dialect, FunctionDefinition& _function, + bool _allowMSizeOptimization, set const& _externallyUsedFunctions ) { while (true) { - UnusedPruner pruner(_dialect, _function, _externallyUsedFunctions); + UnusedPruner pruner(_dialect, _function, _allowMSizeOptimization, _externallyUsedFunctions); pruner(_function); if (!pruner.shouldRunAgain()) return; diff --git a/libyul/optimiser/UnusedPruner.h b/libyul/optimiser/UnusedPruner.h index 02a25efb5..04c43905f 100644 --- a/libyul/optimiser/UnusedPruner.h +++ b/libyul/optimiser/UnusedPruner.h @@ -44,11 +44,13 @@ public: UnusedPruner( Dialect const& _dialect, Block& _ast, + bool _allowMSizeOptimization, std::set const& _externallyUsedFunctions = {} ); UnusedPruner( Dialect const& _dialect, FunctionDefinition& _function, + bool _allowMSizeOptimization, std::set const& _externallyUsedFunctions = {} ); @@ -59,6 +61,13 @@ public: bool shouldRunAgain() const { return m_shouldRunAgain; } // Run the pruner until the code does not change anymore. + static void runUntilStabilised( + Dialect const& _dialect, + Block& _ast, + bool _allowMSizeOptization, + std::set const& _externallyUsedFunctions = {} + ); + static void runUntilStabilised( Dialect const& _dialect, Block& _ast, @@ -67,9 +76,13 @@ public: // Run the pruner until the code does not change anymore. // Only run on the given function. + // @param _allowMSizeOptimization if true, allows to remove instructions + // whose only side-effect is a potential change of the return value of + // the msize instruction. static void runUntilStabilised( Dialect const& _dialect, FunctionDefinition& _functionDefinition, + bool _allowMSizeOptimization, std::set const& _externallyUsedFunctions = {} ); @@ -78,6 +91,7 @@ private: void subtractReferences(std::map const& _subtrahend); Dialect const& m_dialect; + bool m_allowMSizeOptimization = false; bool m_shouldRunAgain = false; std::map m_references; }; diff --git a/test/libyul/yulOptimizerTests/unusedPruner/msize.yul b/test/libyul/yulOptimizerTests/unusedPruner/msize.yul new file mode 100644 index 000000000..ae6beaa75 --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedPruner/msize.yul @@ -0,0 +1,12 @@ +{ + let a := 1 + let b := mload(10) + sstore(0, msize()) +} +// ==== +// step: unusedPruner +// ---- +// { +// pop(mload(10)) +// sstore(0, msize()) +// } diff --git a/test/libyul/yulOptimizerTests/unusedPruner/no_msize.yul b/test/libyul/yulOptimizerTests/unusedPruner/no_msize.yul new file mode 100644 index 000000000..8d40256eb --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedPruner/no_msize.yul @@ -0,0 +1,9 @@ +{ + let a := 1 + let b := mload(10) + sstore(0, 5) +} +// ==== +// step: unusedPruner +// ---- +// { sstore(0, 5) } From df96648b1c4148f0dbea6ff862a0025dfe89466c Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 27 May 2019 14:01:53 +0200 Subject: [PATCH 3/4] Do not allow msize in inline assembly if the Yul optimizer is active. --- libsolidity/analysis/SyntaxChecker.cpp | 20 +++++++++++++++++++ libsolidity/analysis/SyntaxChecker.h | 10 +++++++++- libsolidity/interface/CompilerStack.cpp | 2 +- test/libsolidity/SolidityEndToEndTest.cpp | 15 ++++++++++---- test/libsolidity/SyntaxTest.cpp | 12 ++++++++++- test/libsolidity/SyntaxTest.h | 1 + .../use_msize_with_optimizer.sol | 12 +++++++++++ .../use_msize_without_optimizer.sol | 8 ++++++++ .../yulOptimizerTests/unusedPruner/keccak.yul | 12 +++++++++++ 9 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/use_msize_with_optimizer.sol create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/use_msize_without_optimizer.sol create mode 100644 test/libyul/yulOptimizerTests/unusedPruner/keccak.yul diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index 5f108c6f9..5402d7ff4 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -21,6 +21,9 @@ #include #include +#include +#include + #include #include @@ -254,6 +257,23 @@ bool SyntaxChecker::visit(UnaryOperation const& _operation) return true; } +bool SyntaxChecker::visit(InlineAssembly const& _inlineAssembly) +{ + if (!m_useYulOptimizer) + return false; + + if (yul::SideEffectsCollector( + _inlineAssembly.dialect(), + _inlineAssembly.operations() + ).containsMSize()) + m_errorReporter.syntaxError( + _inlineAssembly.location(), + "The msize instruction cannot be used when the Yul optimizer is activated because " + "it can change its semantics. Either disable the Yul optimizer or do not use the instruction." + ); + return false; +} + bool SyntaxChecker::visit(PlaceholderStatement const&) { m_placeholderFound = true; diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index c24639555..bf2cf9ff6 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -39,12 +39,16 @@ namespace solidity * - whether a modifier contains at least one '_' * - issues deprecation warnings for unary '+' * - issues deprecation warning for throw + * - whether the msize instruction is used and the Yul optimizer is enabled at the same time. */ class SyntaxChecker: private ASTConstVisitor { public: /// @param _errorReporter provides the error logging functionality. - SyntaxChecker(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) {} + SyntaxChecker(langutil::ErrorReporter& _errorReporter, bool _useYulOptimizer): + m_errorReporter(_errorReporter), + m_useYulOptimizer(_useYulOptimizer) + {} bool checkSyntax(ASTNode const& _astRoot); @@ -75,6 +79,8 @@ private: bool visit(UnaryOperation const& _operation) override; + bool visit(InlineAssembly const& _inlineAssembly) override; + bool visit(PlaceholderStatement const& _placeholderStatement) override; bool visit(ContractDefinition const& _contract) override; @@ -88,6 +94,8 @@ private: langutil::ErrorReporter& m_errorReporter; + bool m_useYulOptimizer = false; + /// Flag that indicates whether a function modifier actually contains '_'. bool m_placeholderFound = false; diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index d6e998644..2910a9d95 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -250,7 +250,7 @@ bool CompilerStack::analyze() bool noErrors = true; try { - SyntaxChecker syntaxChecker(m_errorReporter); + SyntaxChecker syntaxChecker(m_errorReporter, m_optimiserSettings.runYulOptimiser); for (Source const* source: m_sourceOrder) if (!syntaxChecker.checkSyntax(*source->ast)) noErrors = false; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index a6b304734..7ca924ac1 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -10082,8 +10082,11 @@ BOOST_AUTO_TEST_CASE(create_memory_array_allocation_size) } } )"; - compileAndRun(sourceCode); - ABI_CHECK(callContractFunction("f()"), encodeArgs(0x40, 0x40, 0x20 + 256, 0x260)); + if (!m_optimiserSettings.runYulOptimiser) + { + compileAndRun(sourceCode); + ABI_CHECK(callContractFunction("f()"), encodeArgs(0x40, 0x40, 0x20 + 256, 0x260)); + } } BOOST_AUTO_TEST_CASE(memory_arrays_of_various_sizes) @@ -11273,8 +11276,12 @@ BOOST_AUTO_TEST_CASE(correctly_initialize_memory_array_in_constructor) } } )"; - compileAndRun(sourceCode, 0, "C"); - ABI_CHECK(callContractFunction("success()"), encodeArgs(u256(1))); + // Cannot run against yul optimizer because of msize + if (!m_optimiserSettings.runYulOptimiser) + { + compileAndRun(sourceCode, 0, "C"); + ABI_CHECK(callContractFunction("success()"), encodeArgs(u256(1))); + } } BOOST_AUTO_TEST_CASE(return_does_not_skip_modifier) diff --git a/test/libsolidity/SyntaxTest.cpp b/test/libsolidity/SyntaxTest.cpp index e139b89c0..61d1d20e6 100644 --- a/test/libsolidity/SyntaxTest.cpp +++ b/test/libsolidity/SyntaxTest.cpp @@ -60,6 +60,12 @@ SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion file.exceptions(ios::badbit); m_source = parseSourceAndSettings(file); + if (m_settings.count("optimize-yul")) + { + m_optimiseYul = true; + m_validatedSettings["optimize-yul"] = "true"; + m_settings.erase("optimize-yul"); + } m_expectations = parseExpectations(file); } @@ -69,7 +75,11 @@ TestCase::TestResult SyntaxTest::run(ostream& _stream, string const& _linePrefix compiler().reset(); compiler().setSources({{"", versionPragma + m_source}}); compiler().setEVMVersion(m_evmVersion); - + compiler().setOptimiserSettings( + m_optimiseYul ? + OptimiserSettings::full() : + OptimiserSettings::minimal() + ); if (compiler().parse()) compiler().analyze(); diff --git a/test/libsolidity/SyntaxTest.h b/test/libsolidity/SyntaxTest.h index 3b89b5d2b..37a078107 100644 --- a/test/libsolidity/SyntaxTest.h +++ b/test/libsolidity/SyntaxTest.h @@ -82,6 +82,7 @@ protected: std::string m_source; std::vector m_expectations; std::vector m_errorList; + bool m_optimiseYul = false; langutil::EVMVersion const m_evmVersion; }; diff --git a/test/libsolidity/syntaxTests/inlineAssembly/use_msize_with_optimizer.sol b/test/libsolidity/syntaxTests/inlineAssembly/use_msize_with_optimizer.sol new file mode 100644 index 000000000..eeba55a4c --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/use_msize_with_optimizer.sol @@ -0,0 +1,12 @@ +contract C { + function f() pure public { + assembly { + let x := msize() + } + } +} +// ==== +// optimize-yul: true +// ---- +// Warning: The Yul optimiser is still experimental. Do not use it in production unless correctness of generated code is verified with extensive tests. +// SyntaxError: (52-101): The msize instruction cannot be used when the Yul optimizer is activated because it can change its semantics. Either disable the Yul optimizer or do not use the instruction. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/use_msize_without_optimizer.sol b/test/libsolidity/syntaxTests/inlineAssembly/use_msize_without_optimizer.sol new file mode 100644 index 000000000..e994d017d --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/use_msize_without_optimizer.sol @@ -0,0 +1,8 @@ +contract C { + function f() pure public { + assembly { + let x := msize() + } + } +} +// ---- diff --git a/test/libyul/yulOptimizerTests/unusedPruner/keccak.yul b/test/libyul/yulOptimizerTests/unusedPruner/keccak.yul new file mode 100644 index 000000000..c6b93a648 --- /dev/null +++ b/test/libyul/yulOptimizerTests/unusedPruner/keccak.yul @@ -0,0 +1,12 @@ +{ + let a := 1 + let b := keccak256(1, 1) + sstore(0, msize()) +} +// ==== +// step: unusedPruner +// ---- +// { +// pop(keccak256(1, 1)) +// sstore(0, msize()) +// } From 010bb395485c3981ec02c1e1f04af41a3c8ce1aa Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 27 May 2019 14:02:53 +0200 Subject: [PATCH 4/4] Changelog entry. --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 12b2e848d..c7fc3ab40 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ Language Features: Compiler Features: * Assembler: Encode the compiler version in the deployed bytecode. * Code Generator: Fix handling of structs of dynamic size as constructor parameters. + * Inline Assembly: Disallow the combination of ``msize()`` and the Yul optimizer. * Metadata: Add IPFS hashes of source files. * Optimizer: Add rule to simplify SHL/SHR combinations. * Optimizer: Add rules for multiplication and division by left-shifted one. @@ -17,6 +18,7 @@ Compiler Features: * SMTChecker: Inline external function calls to ``this``. * Yul Optimizer: Simplify single-run ``for`` loops to ``if`` statements. * Yul Optimizer: Do not inline recursive functions. + * Yul Optimizer: Do not remove instructions that affect ``msize()`` if ``msize()`` is used. Bugfixes: