From 5203503583b637d9917f2ae6fb892cb11751864d Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Mon, 6 Apr 2020 14:47:44 +0200 Subject: [PATCH] Allow for per-parameter literalValues builtin functions --- libyul/AsmAnalysis.cpp | 10 +++--- libyul/Dialect.h | 5 +-- libyul/backends/evm/EVMDialect.cpp | 21 ++++++----- libyul/backends/evm/EVMDialect.h | 2 +- libyul/backends/wasm/WasmCodeTransform.cpp | 10 ++++-- libyul/backends/wasm/WasmDialect.cpp | 13 ++++--- libyul/backends/wasm/WasmDialect.h | 2 +- libyul/backends/wasm/WordSizeTransform.cpp | 36 +++++++++---------- libyul/backends/wasm/WordSizeTransform.h | 1 - .../CommonSubexpressionEliminator.cpp | 15 ++++++-- libyul/optimiser/ExpressionSplitter.cpp | 10 +++--- test/libyul/Parser.cpp | 2 +- 12 files changed, 75 insertions(+), 52 deletions(-) diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 91f818100..ca3f88c75 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -255,14 +255,14 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) yulAssert(!_funCall.functionName.name.empty(), ""); vector const* parameterTypes = nullptr; vector const* returnTypes = nullptr; - bool needsLiteralArguments = false; + vector const* needsLiteralArguments = nullptr; if (BuiltinFunction const* f = m_dialect.builtin(_funCall.functionName.name)) { parameterTypes = &f->parameters; returnTypes = &f->returns; if (f->literalArguments) - needsLiteralArguments = true; + needsLiteralArguments = &f->literalArguments.value(); } else if (!m_currentScope->lookup(_funCall.functionName.name, GenericVisitor{ [&](Scope::Variable const&) @@ -293,11 +293,13 @@ vector AsmAnalyzer::operator()(FunctionCall const& _funCall) ); vector argTypes; - for (auto const& arg: _funCall.arguments | boost::adaptors::reversed) + for (size_t i = _funCall.arguments.size(); i > 0; i--) { + Expression const& arg = _funCall.arguments[i - 1]; + argTypes.emplace_back(expectExpression(arg)); - if (needsLiteralArguments) + if (needsLiteralArguments && (*needsLiteralArguments)[i - 1]) { if (!holds_alternative(arg)) typeError( diff --git a/libyul/Dialect.h b/libyul/Dialect.h index a2a596177..870205833 100644 --- a/libyul/Dialect.h +++ b/libyul/Dialect.h @@ -28,6 +28,7 @@ #include #include +#include namespace solidity::yul { @@ -46,8 +47,8 @@ struct BuiltinFunction ControlFlowSideEffects controlFlowSideEffects; /// 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; + /// If set, same length as the arguments, if true at index i, the i'th argument has to be a literal which means it can't be moved to variables. + std::optional> literalArguments; }; struct Dialect: boost::noncopyable diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index ea6586323..52cd1d23f 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -55,7 +55,7 @@ pair createEVMFunction( f.controlFlowSideEffects.terminates = evmasm::SemanticInformation::terminatesControlFlow(_instruction); f.controlFlowSideEffects.reverts = evmasm::SemanticInformation::reverts(_instruction); f.isMSize = _instruction == evmasm::Instruction::MSIZE; - f.literalArguments = false; + f.literalArguments.reset(); f.instruction = _instruction; f.generateCode = [_instruction]( FunctionCall const&, @@ -75,17 +75,22 @@ pair createFunction( size_t _params, size_t _returns, SideEffects _sideEffects, - bool _literalArguments, + vector _literalArguments, std::function)> _generateCode ) { + solAssert(_literalArguments.size() == _params || _literalArguments.empty(), ""); + YulString name{std::move(_name)}; BuiltinFunctionForEVM f; f.name = name; f.parameters.resize(_params); f.returns.resize(_returns); f.sideEffects = std::move(_sideEffects); - f.literalArguments = _literalArguments; + if (!_literalArguments.empty()) + f.literalArguments = std::move(_literalArguments); + else + f.literalArguments.reset(); f.isMSize = false; f.instruction = {}; f.generateCode = std::move(_generateCode); @@ -107,7 +112,7 @@ map createBuiltins(langutil::EVMVersion _evmVe if (_objectAccess) { - builtins.emplace(createFunction("datasize", 1, 1, SideEffects{}, true, []( + builtins.emplace(createFunction("datasize", 1, 1, SideEffects{}, {true}, []( FunctionCall const& _call, AbstractAssembly& _assembly, BuiltinContext& _context, @@ -128,7 +133,7 @@ map createBuiltins(langutil::EVMVersion _evmVe _assembly.appendDataSize(_context.subIDs.at(dataName)); } })); - builtins.emplace(createFunction("dataoffset", 1, 1, SideEffects{}, true, []( + builtins.emplace(createFunction("dataoffset", 1, 1, SideEffects{}, {true}, []( FunctionCall const& _call, AbstractAssembly& _assembly, BuiltinContext& _context, @@ -154,7 +159,7 @@ map createBuiltins(langutil::EVMVersion _evmVe 3, 0, SideEffects{false, false, false, false, true}, - false, + {}, []( FunctionCall const&, AbstractAssembly& _assembly, @@ -262,7 +267,7 @@ EVMDialectTyped::EVMDialectTyped(langutil::EVMVersion _evmVersion, bool _objectA m_functions["popbool"_yulstring] = m_functions["pop"_yulstring]; m_functions["popbool"_yulstring].name = "popbool"_yulstring; m_functions["popbool"_yulstring].parameters = {"bool"_yulstring}; - m_functions.insert(createFunction("bool_to_u256", 1, 1, {}, false, []( + m_functions.insert(createFunction("bool_to_u256", 1, 1, {}, {}, []( FunctionCall const&, AbstractAssembly&, BuiltinContext&, @@ -272,7 +277,7 @@ EVMDialectTyped::EVMDialectTyped(langutil::EVMVersion _evmVersion, bool _objectA })); m_functions["bool_to_u256"_yulstring].parameters = {"bool"_yulstring}; m_functions["bool_to_u256"_yulstring].returns = {"u256"_yulstring}; - m_functions.insert(createFunction("u256_to_bool", 1, 1, {}, false, []( + m_functions.insert(createFunction("u256_to_bool", 1, 1, {}, {}, []( FunctionCall const&, AbstractAssembly& _assembly, BuiltinContext&, diff --git a/libyul/backends/evm/EVMDialect.h b/libyul/backends/evm/EVMDialect.h index 2141ab98f..00852fe34 100644 --- a/libyul/backends/evm/EVMDialect.h +++ b/libyul/backends/evm/EVMDialect.h @@ -45,7 +45,7 @@ struct BuiltinContext std::map subIDs; }; -struct BuiltinFunctionForEVM: BuiltinFunction +struct BuiltinFunctionForEVM: public BuiltinFunction { std::optional instruction; /// Function to generate code for the given function call and append it to the abstract diff --git a/libyul/backends/wasm/WasmCodeTransform.cpp b/libyul/backends/wasm/WasmCodeTransform.cpp index f3ff04cd9..cb1294d44 100644 --- a/libyul/backends/wasm/WasmCodeTransform.cpp +++ b/libyul/backends/wasm/WasmCodeTransform.cpp @@ -136,11 +136,15 @@ wasm::Expression WasmCodeTransform::operator()(FunctionCall const& _call) } typeConversionNeeded = true; } - else if (builtin->literalArguments) + else if (builtin->literalArguments && contains(builtin->literalArguments.value(), true)) { vector literals; - for (auto const& arg: _call.arguments) - literals.emplace_back(wasm::StringLiteral{std::get(arg).value.str()}); + for (size_t i = 0; i < _call.arguments.size(); i++) + if (builtin->literalArguments.value()[i]) + literals.emplace_back(wasm::StringLiteral{std::get(_call.arguments[i]).value.str()}); + else + literals.emplace_back(visitReturnByValue(_call.arguments[i])); + return wasm::BuiltinCall{_call.functionName.name.str(), std::move(literals)}; } else diff --git a/libyul/backends/wasm/WasmDialect.cpp b/libyul/backends/wasm/WasmDialect.cpp index c9817af76..c9dfe05c7 100644 --- a/libyul/backends/wasm/WasmDialect.cpp +++ b/libyul/backends/wasm/WasmDialect.cpp @@ -102,8 +102,8 @@ WasmDialect::WasmDialect() m_functions["unreachable"_yulstring].controlFlowSideEffects.terminates = true; m_functions["unreachable"_yulstring].controlFlowSideEffects.reverts = true; - addFunction("datasize", {i64}, {i64}, true, true); - addFunction("dataoffset", {i64}, {i64}, true, true); + addFunction("datasize", {i64}, {i64}, true, {true}); + addFunction("dataoffset", {i64}, {i64}, true, {true}); addEthereumExternals(); } @@ -204,7 +204,7 @@ void WasmDialect::addEthereumExternals() f.controlFlowSideEffects = ext.controlFlowSideEffects; f.isMSize = false; f.sideEffects.invalidatesStorage = (ext.name == "storageStore"); - f.literalArguments = false; + f.literalArguments.reset(); } } @@ -213,7 +213,7 @@ void WasmDialect::addFunction( vector _params, vector _returns, bool _movable, - bool _literalArguments + std::vector _literalArguments ) { YulString name{move(_name)}; @@ -224,5 +224,8 @@ void WasmDialect::addFunction( f.returns = std::move(_returns); f.sideEffects = _movable ? SideEffects{} : SideEffects::worst(); f.isMSize = false; - f.literalArguments = _literalArguments; + if (!_literalArguments.empty()) + f.literalArguments = std::move(_literalArguments); + else + f.literalArguments.reset(); } diff --git a/libyul/backends/wasm/WasmDialect.h b/libyul/backends/wasm/WasmDialect.h index 1de65dfdc..e5f3c9fc4 100644 --- a/libyul/backends/wasm/WasmDialect.h +++ b/libyul/backends/wasm/WasmDialect.h @@ -61,7 +61,7 @@ private: std::vector _params, std::vector _returns, bool _movable = true, - bool _literalArguments = false + std::vector _literalArguments = std::vector{} ); std::map m_functions; diff --git a/libyul/backends/wasm/WordSizeTransform.cpp b/libyul/backends/wasm/WordSizeTransform.cpp index eec04fae6..bc450a8e4 100644 --- a/libyul/backends/wasm/WordSizeTransform.cpp +++ b/libyul/backends/wasm/WordSizeTransform.cpp @@ -41,15 +41,24 @@ void WordSizeTransform::operator()(FunctionDefinition& _fd) void WordSizeTransform::operator()(FunctionCall& _fc) { + vector const* literalArguments = nullptr; + if (BuiltinFunction const* fun = m_inputDialect.builtin(_fc.functionName.name)) if (fun->literalArguments) + literalArguments = &fun->literalArguments.value(); + + vector newArgs; + + for (size_t i = 0; i < _fc.arguments.size(); i++) + if (!literalArguments || !(*literalArguments)[i]) + newArgs += expandValueToVector(_fc.arguments[i]); + else { - for (Expression& arg: _fc.arguments) - get(arg).type = m_targetDialect.defaultType; - return; + get(_fc.arguments[i]).type = m_targetDialect.defaultType; + newArgs.emplace_back(std::move(_fc.arguments[i])); } - rewriteFunctionCallArguments(_fc.arguments); + _fc.arguments = std::move(newArgs); } void WordSizeTransform::operator()(If& _if) @@ -97,9 +106,9 @@ void WordSizeTransform::operator()(Block& _block) // Special handling for datasize and dataoffset - they will only need one variable. if (BuiltinFunction const* f = m_inputDialect.builtin(std::get(*varDecl.value).functionName.name)) - if (f->literalArguments) + if (f->name == "datasize"_yulstring || f->name == "dataoffset"_yulstring) { - yulAssert(f->name == "datasize"_yulstring || f->name == "dataoffset"_yulstring, ""); + yulAssert(f->literalArguments && f->literalArguments.value()[0], ""); yulAssert(varDecl.variables.size() == 1, ""); auto newLhs = generateU64IdentifierNames(varDecl.variables[0].name); vector ret; @@ -158,9 +167,9 @@ void WordSizeTransform::operator()(Block& _block) // Special handling for datasize and dataoffset - they will only need one variable. if (BuiltinFunction const* f = m_inputDialect.builtin(std::get(*assignment.value).functionName.name)) - if (f->literalArguments) + if (f->name == "datasize"_yulstring || f->name == "dataoffset"_yulstring) { - yulAssert(f->name == "datasize"_yulstring || f->name == "dataoffset"_yulstring, ""); + yulAssert(f->literalArguments && f->literalArguments.value()[0], ""); yulAssert(assignment.variableNames.size() == 1, ""); auto newLhs = generateU64IdentifierNames(assignment.variableNames[0].name); vector ret; @@ -268,17 +277,6 @@ void WordSizeTransform::rewriteIdentifierList(vector& _ids) ); } -void WordSizeTransform::rewriteFunctionCallArguments(vector& _args) -{ - iterateReplacing( - _args, - [&](Expression& _e) -> std::optional> - { - return expandValueToVector(_e); - } - ); -} - vector WordSizeTransform::handleSwitchInternal( langutil::SourceLocation const& _location, vector const& _splitExpressions, diff --git a/libyul/backends/wasm/WordSizeTransform.h b/libyul/backends/wasm/WordSizeTransform.h index 67dfe8864..5771afea8 100644 --- a/libyul/backends/wasm/WordSizeTransform.h +++ b/libyul/backends/wasm/WordSizeTransform.h @@ -83,7 +83,6 @@ private: void rewriteVarDeclList(std::vector&); void rewriteIdentifierList(std::vector&); - void rewriteFunctionCallArguments(std::vector&); std::vector handleSwitch(Switch& _switch); std::vector handleSwitchInternal( diff --git a/libyul/optimiser/CommonSubexpressionEliminator.cpp b/libyul/optimiser/CommonSubexpressionEliminator.cpp index 84074ad27..3b0b6bec1 100644 --- a/libyul/optimiser/CommonSubexpressionEliminator.cpp +++ b/libyul/optimiser/CommonSubexpressionEliminator.cpp @@ -58,12 +58,21 @@ void CommonSubexpressionEliminator::visit(Expression& _e) // If this is a function call to a function that requires literal arguments, // do not try to simplify there. if (holds_alternative(_e)) - if (BuiltinFunction const* builtin = m_dialect.builtin(std::get(_e).functionName.name)) - if (builtin->literalArguments) + { + FunctionCall& funCall = std::get(_e); + + if (BuiltinFunction const* builtin = m_dialect.builtin(funCall.functionName.name)) + { + for (size_t i = funCall.arguments.size(); i > 0; i--) // 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; + if (!builtin->literalArguments || !builtin->literalArguments.value()[i - 1]) + visit(funCall.arguments[i - 1]); + + descend = false; + } + } // We visit the inner expression first to first simplify inner expressions, // which hopefully allows more matches. diff --git a/libyul/optimiser/ExpressionSplitter.cpp b/libyul/optimiser/ExpressionSplitter.cpp index 7b2ac14f6..8144de49a 100644 --- a/libyul/optimiser/ExpressionSplitter.cpp +++ b/libyul/optimiser/ExpressionSplitter.cpp @@ -47,13 +47,15 @@ void ExpressionSplitter::run(OptimiserStepContext& _context, Block& _ast) void ExpressionSplitter::operator()(FunctionCall& _funCall) { + vector const* literalArgs = nullptr; + if (BuiltinFunction const* builtin = m_dialect.builtin(_funCall.functionName.name)) if (builtin->literalArguments) - // We cannot outline function arguments that have to be literals - return; + literalArgs = &builtin->literalArguments.value(); - for (auto& arg: _funCall.arguments | boost::adaptors::reversed) - outlineExpression(arg); + for (size_t i = _funCall.arguments.size(); i > 0; i--) + if (!literalArgs || !(*literalArgs)[i - 1]) + outlineExpression(_funCall.arguments[i - 1]); } void ExpressionSplitter::operator()(If& _if) diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index 93b528721..7b8e56119 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -539,7 +539,7 @@ BOOST_AUTO_TEST_CASE(builtins_analysis) { return _name == "builtin"_yulstring ? &f : nullptr; } - BuiltinFunction f{"builtin"_yulstring, vector(2), vector(3), {}, {}}; + BuiltinFunction f{"builtin"_yulstring, vector(2), vector(3), {}, {}, false, {}}; }; SimpleDialect dialect;