diff options
-rw-r--r-- | Changelog.md | 1 | ||||
-rw-r--r-- | libyul/AsmAnalysis.cpp | 10 | ||||
-rw-r--r-- | libyul/Dialect.h | 8 | ||||
-rw-r--r-- | libyul/backends/evm/EVMDialect.cpp | 8 | ||||
-rw-r--r-- | libyul/backends/evm/EVMDialect.h | 1 | ||||
-rw-r--r-- | libyul/optimiser/CommonSubexpressionEliminator.cpp | 15 | ||||
-rw-r--r-- | libyul/optimiser/CommonSubexpressionEliminator.h | 2 | ||||
-rw-r--r-- | libyul/optimiser/ExpressionSplitter.cpp | 6 | ||||
-rw-r--r-- | libyul/optimiser/ExpressionSplitter.h | 6 | ||||
-rw-r--r-- | libyul/optimiser/Suite.cpp | 4 | ||||
-rw-r--r-- | libyul/optimiser/Suite.h | 1 | ||||
-rw-r--r-- | test/libyul/ObjectParser.cpp | 30 | ||||
-rw-r--r-- | test/libyul/Parser.cpp | 2 | ||||
-rw-r--r-- | test/libyul/YulOptimizerTest.cpp | 10 | ||||
-rw-r--r-- | test/libyul/yulOptimizerTests/commonSubexpressionEliminator/object_access.yul | 23 | ||||
-rw-r--r-- | test/libyul/yulOptimizerTests/expressionSplitter/object_access.yul | 21 | ||||
-rw-r--r-- | test/tools/yulopti.cpp | 2 |
17 files changed, 133 insertions, 17 deletions
diff --git a/Changelog.md b/Changelog.md index 3d298f6f..23e394c6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ Compiler Features: Bugfixes: + * Yul: Check that arguments to ``dataoffset`` and ``datasize`` are literals at parse time and properly take this into account in the optimizer. Build System: diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 0ecc5a30..6a7b2b61 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 01fd98df..e06a6826 100644 --- a/libyul/Dialect.h +++ b/libyul/Dialect.h @@ -44,7 +44,13 @@ struct BuiltinFunction YulString name; std::vector<Type> parameters; std::vector<Type> 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 935f05c6..18502117 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<void()> @@ -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<void()> @@ -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<void()> _visitArguments @@ -128,6 +128,7 @@ void EVMDialect::addFunction( size_t _params, size_t _returns, bool _movable, + bool _literalArguments, std::function<void(FunctionCall const&, AbstractAssembly&, std::function<void()>)> _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 feb00b03..c23833bc 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<void(FunctionCall const&, AbstractAssembly&, std::function<void()>)> _generateCode ); diff --git a/libyul/optimiser/CommonSubexpressionEliminator.cpp b/libyul/optimiser/CommonSubexpressionEliminator.cpp index 9b851333..8ce003e7 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.cpp +++ b/libyul/optimiser/CommonSubexpressionEliminator.cpp @@ -25,6 +25,7 @@ #include <libyul/optimiser/SyntacticalEquality.h> #include <libyul/Exceptions.h> #include <libyul/AsmData.h> +#include <libyul/Dialect.h> 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<FunctionCall>(_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 4b7973e2..9f416d9f 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 a3b2dc11..334e33f4 100644 --- a/libyul/optimiser/ExpressionSplitter.cpp +++ b/libyul/optimiser/ExpressionSplitter.cpp @@ -24,6 +24,7 @@ #include <libyul/optimiser/ASTWalker.h> #include <libyul/AsmData.h> +#include <libyul/Dialect.h> #include <libdevcore/CommonData.h> @@ -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 d4d2b3f6..a72d14ba 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<Statement> m_statementsToPrefix; + Dialect const& m_dialect; NameDispenser& m_nameDispenser; }; diff --git a/libyul/optimiser/Suite.cpp b/libyul/optimiser/Suite.cpp index 9b6e1337..e10916ea 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 ce40b204..376e9889 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<YulString> const& _externallyUsedIdentifiers = {} ); }; diff --git a/test/libyul/ObjectParser.cpp b/test/libyul/ObjectParser.cpp index bb88e4da..13a95788 100644 --- a/test/libyul/ObjectParser.cpp +++ b/test/libyul/ObjectParser.cpp @@ -250,6 +250,36 @@ BOOST_AUTO_TEST_CASE(to_string) BOOST_CHECK_EQUAL(asmStack.print(), expectation); } +BOOST_AUTO_TEST_CASE(arg_to_dataoffset_must_be_literal) +{ + string code = R"( + object "outer" { + code { let x := "outer" let y := dataoffset(x) } + } + )"; + CHECK_ERROR(code, TypeError, "Function expects direct literals as arguments."); +} + +BOOST_AUTO_TEST_CASE(arg_to_datasize_must_be_literal) +{ + string code = R"( + object "outer" { + code { let x := "outer" let y := datasize(x) } + } + )"; + CHECK_ERROR(code, TypeError, "Function expects direct literals as arguments."); +} + +BOOST_AUTO_TEST_CASE(args_to_datacopy_are_arbitrary) +{ + string code = R"( + object "outer" { + code { let x := 0 let y := 2 let s := 3 datacopy(x, y, s) } + } + )"; + BOOST_CHECK(successParse(code)); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index df7e32a1..897f18ae 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<Type>(2), vector<Type>(3), false}; + BuiltinFunction f{"builtin"_yulstring, vector<Type>(2), vector<Type>(3), false, false}; }; shared_ptr<Dialect> dialect = make_shared<SimpleDialect>(); diff --git a/test/libyul/YulOptimizerTest.cpp b/test/libyul/YulOptimizerTest.cpp index 1b392e0f..0e3203cb 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> scanner = make_shared<Scanner>(CharStream(m_source, "")); diff --git a/test/libyul/yulOptimizerTests/commonSubexpressionEliminator/object_access.yul b/test/libyul/yulOptimizerTests/commonSubexpressionEliminator/object_access.yul new file mode 100644 index 00000000..5cfa3e6e --- /dev/null +++ b/test/libyul/yulOptimizerTests/commonSubexpressionEliminator/object_access.yul @@ -0,0 +1,23 @@ +{ + // Arguments to ``datasize`` and ``dataoffset`` need to be + // literals. We cannot simplify their arguments, but we can + // simplify them as a full expression. + // ``datacopy`` does not have this restriction. + let r := "abc" + let a := datasize("abc") + let x := dataoffset("abc") + // should be replaced by a + let y := datasize("abc") + datacopy("abc", x, y) + mstore(a, x) +} +// ---- +// commonSubexpressionEliminator +// { +// let r := "abc" +// let a := datasize("abc") +// let x := dataoffset("abc") +// let y := a +// datacopy(r, x, a) +// mstore(a, x) +// } diff --git a/test/libyul/yulOptimizerTests/expressionSplitter/object_access.yul b/test/libyul/yulOptimizerTests/expressionSplitter/object_access.yul new file mode 100644 index 00000000..2689ab6f --- /dev/null +++ b/test/libyul/yulOptimizerTests/expressionSplitter/object_access.yul @@ -0,0 +1,21 @@ +{ + // We should never split arguments to ``dataoffset`` + // or ``datasize`` because they need to be literals + let x := dataoffset("abc") + let y := datasize("abc") + // datacopy is fine, though + datacopy(mload(0), mload(1), mload(2)) +} +// ---- +// expressionSplitter +// { +// let x := dataoffset("abc") +// let y := datasize("abc") +// let _1 := 2 +// let _2 := mload(_1) +// let _3 := 1 +// let _4 := mload(_3) +// let _5 := 0 +// let _6 := mload(_5) +// datacopy(_6, _4, _2) +// } diff --git a/test/tools/yulopti.cpp b/test/tools/yulopti.cpp index 8ef56ffe..ac21fd91 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); |