From 5b73c2ae3bce09442572b5401a7bcccc2ffe7590 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 20 Dec 2018 17:22:17 +0100 Subject: [PATCH] Take special functions that require literals into account. --- libyul/AsmAnalysis.cpp | 10 ++++++++++ libyul/Dialect.h | 8 +++++++- libyul/backends/evm/EVMDialect.cpp | 8 +++++--- libyul/backends/evm/EVMDialect.h | 1 + .../optimiser/CommonSubexpressionEliminator.cpp | 15 ++++++++++++++- libyul/optimiser/CommonSubexpressionEliminator.h | 2 ++ libyul/optimiser/ExpressionSplitter.cpp | 6 ++++++ libyul/optimiser/ExpressionSplitter.h | 6 ++++-- libyul/optimiser/Suite.cpp | 4 ++-- libyul/optimiser/Suite.h | 1 - test/libyul/Parser.cpp | 2 +- test/libyul/YulOptimizerTest.cpp | 10 +++++----- test/tools/yulopti.cpp | 2 +- 13 files changed, 58 insertions(+), 17 deletions(-) diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 0ecc5a30f..6a7b2b61d 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -299,11 +299,14 @@ bool AsmAnalyzer::operator()(FunctionCall const& _funCall) bool success = true; size_t parameters = 0; size_t returns = 0; + bool needsLiteralArguments = false; if (BuiltinFunction const* f = m_dialect->builtin(_funCall.functionName.name)) { // TODO: compare types, too parameters = f->parameters.size(); returns = f->returns.size(); + if (f->literalArguments) + needsLiteralArguments = true; } else if (!m_currentScope->lookup(_funCall.functionName.name, Scope::Visitor( [&](Scope::Variable const&) @@ -347,8 +350,15 @@ bool AsmAnalyzer::operator()(FunctionCall const& _funCall) } for (auto const& arg: _funCall.arguments | boost::adaptors::reversed) + { if (!expectExpression(arg)) success = false; + else if (needsLiteralArguments && arg.type() != typeid(Literal)) + m_errorReporter.typeError( + _funCall.functionName.location, + "Function expects direct literals as arguments." + ); + } // Use argument size instead of parameter count to avoid misleading errors. m_stackHeight += int(returns) - int(_funCall.arguments.size()); m_info.stackHeightInfo[&_funCall] = m_stackHeight; diff --git a/libyul/Dialect.h b/libyul/Dialect.h index 01fd98df6..e06a68263 100644 --- a/libyul/Dialect.h +++ b/libyul/Dialect.h @@ -44,7 +44,13 @@ struct BuiltinFunction YulString name; std::vector parameters; std::vector returns; - bool movable; + /// If true, calls to this function can be freely moved and copied (as long as their + /// arguments are either variables or also movable) without altering the semantics. + /// This means the function cannot depend on storage or memory, cannot have any side-effects, + /// but it can depend on state that is constant across an EVM-call. + bool movable = false; + /// If true, can only accept literals as arguments and they cannot be moved to voriables. + bool literalArguments = false; }; struct Dialect: boost::noncopyable diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index 935f05c6d..185021174 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -44,7 +44,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess): if (!m_objectAccess) return; - addFunction("datasize", 1, 1, true, [this]( + addFunction("datasize", 1, 1, true, true, [this]( FunctionCall const& _call, AbstractAssembly& _assembly, std::function @@ -58,7 +58,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess): else _assembly.appendDataSize(m_subIDs.at(dataName)); }); - addFunction("dataoffset", 1, 1, true, [this]( + addFunction("dataoffset", 1, 1, true, true, [this]( FunctionCall const& _call, AbstractAssembly& _assembly, std::function @@ -72,7 +72,7 @@ EVMDialect::EVMDialect(AsmFlavour _flavour, bool _objectAccess): else _assembly.appendDataOffset(m_subIDs.at(dataName)); }); - addFunction("datacopy", 3, 0, false, []( + addFunction("datacopy", 3, 0, false, false, []( FunctionCall const&, AbstractAssembly& _assembly, std::function _visitArguments @@ -128,6 +128,7 @@ void EVMDialect::addFunction( size_t _params, size_t _returns, bool _movable, + bool _literalArguments, std::function)> _generateCode ) { @@ -137,5 +138,6 @@ void EVMDialect::addFunction( f.parameters.resize(_params); f.returns.resize(_returns); f.movable = _movable; + f.literalArguments = _literalArguments; f.generateCode = std::move(_generateCode); } diff --git a/libyul/backends/evm/EVMDialect.h b/libyul/backends/evm/EVMDialect.h index feb00b038..c23833bcc 100644 --- a/libyul/backends/evm/EVMDialect.h +++ b/libyul/backends/evm/EVMDialect.h @@ -72,6 +72,7 @@ private: size_t _params, size_t _returns, bool _movable, + bool _literalArguments, std::function)> _generateCode ); diff --git a/libyul/optimiser/CommonSubexpressionEliminator.cpp b/libyul/optimiser/CommonSubexpressionEliminator.cpp index 9b8513337..8ce003e70 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.cpp +++ b/libyul/optimiser/CommonSubexpressionEliminator.cpp @@ -25,6 +25,7 @@ #include #include #include +#include using namespace std; using namespace dev; @@ -32,12 +33,24 @@ using namespace yul; void CommonSubexpressionEliminator::visit(Expression& _e) { + bool descend = true; + // If this is a function call to a function that requires literal arguments, + // do not try to simplify there. + if (_e.type() == typeid(FunctionCall)) + if (BuiltinFunction const* builtin = m_dialect.builtin(boost::get(_e).functionName.name)) + if (builtin->literalArguments) + // We should not modify function arguments that have to be literals + // Note that replacing the function call entirely is fine, + // if the function call is movable. + descend = false; + // We visit the inner expression first to first simplify inner expressions, // which hopefully allows more matches. // Note that the DataFlowAnalyzer itself only has code for visiting Statements, // so this basically invokes the AST walker directly and thus post-visiting // is also fine with regards to data flow analysis. - DataFlowAnalyzer::visit(_e); + if (descend) + DataFlowAnalyzer::visit(_e); if (_e.type() == typeid(Identifier)) { diff --git a/libyul/optimiser/CommonSubexpressionEliminator.h b/libyul/optimiser/CommonSubexpressionEliminator.h index 4b7973e2b..9f416d9fd 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.h +++ b/libyul/optimiser/CommonSubexpressionEliminator.h @@ -26,6 +26,8 @@ namespace yul { +struct Dialect; + /** * Optimisation stage that replaces expressions known to be the current value of a variable * in scope by a reference to that variable. diff --git a/libyul/optimiser/ExpressionSplitter.cpp b/libyul/optimiser/ExpressionSplitter.cpp index a3b2dc110..334e33f45 100644 --- a/libyul/optimiser/ExpressionSplitter.cpp +++ b/libyul/optimiser/ExpressionSplitter.cpp @@ -24,6 +24,7 @@ #include #include +#include #include @@ -43,6 +44,11 @@ void ExpressionSplitter::operator()(FunctionalInstruction& _instruction) void ExpressionSplitter::operator()(FunctionCall& _funCall) { + if (BuiltinFunction const* builtin = m_dialect.builtin(_funCall.functionName.name)) + if (builtin->literalArguments) + // We cannot outline function arguments that have to be literals + return; + for (auto& arg: _funCall.arguments | boost::adaptors::reversed) outlineExpression(arg); } diff --git a/libyul/optimiser/ExpressionSplitter.h b/libyul/optimiser/ExpressionSplitter.h index d4d2b3f61..a72d14ba4 100644 --- a/libyul/optimiser/ExpressionSplitter.h +++ b/libyul/optimiser/ExpressionSplitter.h @@ -31,6 +31,7 @@ namespace yul { class NameCollector; +struct Dialect; /** @@ -57,8 +58,8 @@ class NameCollector; class ExpressionSplitter: public ASTModifier { public: - explicit ExpressionSplitter(NameDispenser& _nameDispenser): - m_nameDispenser(_nameDispenser) + explicit ExpressionSplitter(Dialect const& _dialect, NameDispenser& _nameDispenser): + m_dialect(_dialect), m_nameDispenser(_nameDispenser) { } void operator()(FunctionalInstruction&) override; @@ -77,6 +78,7 @@ private: /// List of statements that should go in front of the currently visited AST element, /// at the statement level. std::vector m_statementsToPrefix; + Dialect const& m_dialect; NameDispenser& m_nameDispenser; }; diff --git a/libyul/optimiser/Suite.cpp b/libyul/optimiser/Suite.cpp index 9b6e1337b..e10916ea9 100644 --- a/libyul/optimiser/Suite.cpp +++ b/libyul/optimiser/Suite.cpp @@ -67,7 +67,7 @@ void OptimiserSuite::run( for (size_t i = 0; i < 4; i++) { - ExpressionSplitter{dispenser}(ast); + ExpressionSplitter{_dialect, dispenser}(ast); SSATransform::run(ast, dispenser); RedundantAssignEliminator::run(_dialect, ast); RedundantAssignEliminator::run(_dialect, ast); @@ -90,7 +90,7 @@ void OptimiserSuite::run( ExpressionInliner(_dialect, ast).run(); UnusedPruner::runUntilStabilised(_dialect, ast); - ExpressionSplitter{dispenser}(ast); + ExpressionSplitter{_dialect, dispenser}(ast); SSATransform::run(ast, dispenser); RedundantAssignEliminator::run(_dialect, ast); RedundantAssignEliminator::run(_dialect, ast); diff --git a/libyul/optimiser/Suite.h b/libyul/optimiser/Suite.h index ce40b204a..376e9889b 100644 --- a/libyul/optimiser/Suite.h +++ b/libyul/optimiser/Suite.h @@ -41,7 +41,6 @@ public: Dialect const& _dialect, Block& _ast, AsmAnalysisInfo const& _analysisInfo, - std::set const& _externallyUsedIdentifiers = {} ); }; diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index df7e32a1c..897f18aeb 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -331,7 +331,7 @@ BOOST_AUTO_TEST_CASE(builtins_analysis) { return _name == "builtin"_yulstring ? &f : nullptr; } - BuiltinFunction f{"builtin"_yulstring, vector(2), vector(3), false}; + BuiltinFunction f{"builtin"_yulstring, vector(2), vector(3), false, false}; }; shared_ptr dialect = make_shared(); diff --git a/test/libyul/YulOptimizerTest.cpp b/test/libyul/YulOptimizerTest.cpp index 1b392e0f9..0e3203cbe 100644 --- a/test/libyul/YulOptimizerTest.cpp +++ b/test/libyul/YulOptimizerTest.cpp @@ -122,7 +122,7 @@ bool YulOptimizerTest::run(ostream& _stream, string const& _linePrefix, bool con else if (m_optimizerStep == "expressionSplitter") { NameDispenser nameDispenser{*m_dialect, *m_ast}; - ExpressionSplitter{nameDispenser}(*m_ast); + ExpressionSplitter{*m_dialect, nameDispenser}(*m_ast); } else if (m_optimizerStep == "expressionJoiner") { @@ -133,7 +133,7 @@ bool YulOptimizerTest::run(ostream& _stream, string const& _linePrefix, bool con { disambiguate(); NameDispenser nameDispenser{*m_dialect, *m_ast}; - ExpressionSplitter{nameDispenser}(*m_ast); + ExpressionSplitter{*m_dialect, nameDispenser}(*m_ast); ExpressionJoiner::run(*m_ast); ExpressionJoiner::run(*m_ast); } @@ -158,7 +158,7 @@ bool YulOptimizerTest::run(ostream& _stream, string const& _linePrefix, bool con (FunctionHoister{})(*m_ast); (FunctionGrouper{})(*m_ast); NameDispenser nameDispenser{*m_dialect, *m_ast}; - ExpressionSplitter{nameDispenser}(*m_ast); + ExpressionSplitter{*m_dialect, nameDispenser}(*m_ast); FullInliner(*m_ast, nameDispenser).run(); ExpressionJoiner::run(*m_ast); } @@ -182,7 +182,7 @@ bool YulOptimizerTest::run(ostream& _stream, string const& _linePrefix, bool con { disambiguate(); NameDispenser nameDispenser{*m_dialect, *m_ast}; - ExpressionSplitter{nameDispenser}(*m_ast); + ExpressionSplitter{*m_dialect, nameDispenser}(*m_ast); CommonSubexpressionEliminator{*m_dialect}(*m_ast); ExpressionSimplifier::run(*m_dialect, *m_ast); UnusedPruner::runUntilStabilised(*m_dialect, *m_ast); @@ -260,7 +260,7 @@ void YulOptimizerTest::printIndented(ostream& _stream, string const& _output, st bool YulOptimizerTest::parse(ostream& _stream, string const& _linePrefix, bool const _formatted) { - m_dialect = m_yul ? yul::Dialect::yul() : yul::EVMDialect::strictAssemblyForEVM(); + m_dialect = m_yul ? yul::Dialect::yul() : yul::EVMDialect::strictAssemblyForEVMObjects(); ErrorList errors; ErrorReporter errorReporter(errors); shared_ptr scanner = make_shared(CharStream(m_source, "")); diff --git a/test/tools/yulopti.cpp b/test/tools/yulopti.cpp index 8ef56ffe2..ac21fd91f 100644 --- a/test/tools/yulopti.cpp +++ b/test/tools/yulopti.cpp @@ -149,7 +149,7 @@ public: (VarDeclInitializer{})(*m_ast); break; case 'x': - ExpressionSplitter{*m_nameDispenser}(*m_ast); + ExpressionSplitter{*m_dialect, *m_nameDispenser}(*m_ast); break; case 'j': ExpressionJoiner::run(*m_ast);