From c7986c49170855d3144c8d267197dab61b6f8b37 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 5 Jul 2021 12:02:00 +0200 Subject: [PATCH] Review suggestions. --- libyul/backends/evm/ControlFlowGraph.h | 12 +- .../backends/evm/ControlFlowGraphBuilder.cpp | 221 ++++++++---------- libyul/backends/evm/ControlFlowGraphBuilder.h | 1 + test/libyul/ControlFlowGraphTest.cpp | 2 +- 4 files changed, 113 insertions(+), 123 deletions(-) diff --git a/libyul/backends/evm/ControlFlowGraph.h b/libyul/backends/evm/ControlFlowGraph.h index 46224edae..de6b86b77 100644 --- a/libyul/backends/evm/ControlFlowGraph.h +++ b/libyul/backends/evm/ControlFlowGraph.h @@ -96,7 +96,7 @@ inline bool canBeFreelyGenerated(StackSlot const& _slot) return std::visit([](auto const& _typedSlot) { return std::decay_t::canBeFreelyGenerated; }, _slot); } -/// Control flow graph consisting of ``DFG::BasicBlock``s connected by control flow. +/// Control flow graph consisting of ``CFG::BasicBlock``s connected by control flow. struct CFG { explicit CFG() {} @@ -111,6 +111,8 @@ struct CFG std::reference_wrapper builtin; std::reference_wrapper functionCall; /// Number of proper arguments with a position on the stack, excluding literal arguments. + /// Literal arguments (like the literal string in ``datasize``) do not have a location on the stack, + /// but are handled internally by the builtin's code generation function. size_t arguments = 0; }; struct FunctionCall @@ -179,9 +181,13 @@ struct CFG /// Container for blocks for explicit ownership. std::list blocks; - /// Container for creates variables for explicit ownership. + /// Container for generated variables for explicit ownership. + /// Ghost variables are generated to store switch conditions when transforming the control flow + /// of a switch to a sequence of conditional jumps. std::list ghostVariables; - /// Container for creates calls for explicit ownership. + /// Container for generated calls for explicit ownership. + /// Ghost calls are used for the equality comparisons of the switch condition ghost variable with + /// the switch case literals when transforming the control flow of a switch to a sequence of conditional jumps. std::list ghostCalls; BasicBlock& makeBlock() diff --git a/libyul/backends/evm/ControlFlowGraphBuilder.cpp b/libyul/backends/evm/ControlFlowGraphBuilder.cpp index eaad98cb4..467600898 100644 --- a/libyul/backends/evm/ControlFlowGraphBuilder.cpp +++ b/libyul/backends/evm/ControlFlowGraphBuilder.cpp @@ -62,12 +62,9 @@ std::unique_ptr ControlFlowGraphBuilder::build( // Determine which blocks are reachable from the entry. util::BreadthFirstSearch reachabilityCheck{{result->entry}}; - ranges::actions::push_back( - reachabilityCheck.verticesToTraverse, - result->functionInfo | ranges::views::values | ranges::views::transform( - [](auto&& _function) { return _function.entry; } - ) - ); + for (auto const& functionInfo: result->functionInfo | ranges::views::values) + reachabilityCheck.verticesToTraverse.emplace_back(functionInfo.entry); + reachabilityCheck.run([&](CFG::BasicBlock* _node, auto&& _addChild) { visit(util::GenericVisitor{ [&](CFG::BasicBlock::Jump& _jump) { @@ -100,9 +97,9 @@ ControlFlowGraphBuilder::ControlFlowGraphBuilder( AsmAnalysisInfo& _analysisInfo, Dialect const& _dialect ): -m_graph(_graph), -m_info(_analysisInfo), -m_dialect(_dialect) + m_graph(_graph), + m_info(_analysisInfo), + m_dialect(_dialect) { } @@ -121,58 +118,6 @@ StackSlot ControlFlowGraphBuilder::operator()(Expression const& _expression) return std::visit(*this, _expression); } -CFG::Operation& ControlFlowGraphBuilder::visitFunctionCall(FunctionCall const& _call) -{ - yulAssert(m_scope, ""); - yulAssert(m_currentBlock, ""); - - if (BuiltinFunction const* builtin = m_dialect.builtin(_call.functionName.name)) - { - CFG::Operation& operation = m_currentBlock->operations.emplace_back(CFG::Operation{ - // input - _call.arguments | - ranges::views::enumerate | - ranges::views::reverse | - ranges::views::filter(util::mapTuple([&](size_t idx, auto const&) { - return !builtin->literalArgument(idx).has_value(); - })) | - ranges::views::values | - ranges::views::transform(std::ref(*this)) | - ranges::to, - // output - ranges::views::iota(0u, builtin->returns.size()) | ranges::views::transform([&](size_t _i) { - return TemporarySlot{_call, _i}; - }) | ranges::to, - // operation - CFG::BuiltinCall{_call.debugData, *builtin, _call} - }); - std::get(operation.operation).arguments = operation.input.size(); - return operation; - } - else - { - Scope::Function* function = nullptr; - yulAssert(m_scope->lookup(_call.functionName.name, util::GenericVisitor{ - [](Scope::Variable&) { yulAssert(false, "Expected function name."); }, - [&](Scope::Function& _function) { function = &_function; } - }), "Function name not found."); - yulAssert(function, ""); - return m_currentBlock->operations.emplace_back(CFG::Operation{ - // input - ranges::concat_view( - ranges::views::single(StackSlot{FunctionCallReturnLabelSlot{_call}}), - _call.arguments | ranges::views::reverse | ranges::views::transform(std::ref(*this)) - ) | ranges::to, - // output - ranges::views::iota(0u, function->returns.size()) | ranges::views::transform([&](size_t _i) { - return TemporarySlot{_call, _i}; - }) | ranges::to, - // operation - CFG::FunctionCall{_call.debugData, *function, _call} - }); - } -} - StackSlot ControlFlowGraphBuilder::operator()(FunctionCall const& _call) { CFG::Operation& operation = visitFunctionCall(_call); @@ -188,17 +133,7 @@ void ControlFlowGraphBuilder::operator()(VariableDeclaration const& _varDecl) }) | ranges::to; Stack input; if (_varDecl.value) - input = std::visit(util::GenericVisitor{ - [&](FunctionCall const& _call) -> Stack { - CFG::Operation& operation = visitFunctionCall(_call); - yulAssert(declaredVariables.size() == operation.output.size(), ""); - return operation.output; - }, - [&](auto const& _identifierOrLiteral) -> Stack{ - yulAssert(declaredVariables.size() == 1, ""); - return {(*this)(_identifierOrLiteral)}; - } - }, *_varDecl.value); + input = visitAssignmentRightHandSide(*_varDecl.value, declaredVariables.size()); else input = ranges::views::iota(0u, _varDecl.variables.size()) | ranges::views::transform([&](size_t) { return LiteralSlot{0, _varDecl.debugData}; @@ -218,17 +153,7 @@ void ControlFlowGraphBuilder::operator()(Assignment const& _assignment) yulAssert(m_currentBlock, ""); m_currentBlock->operations.emplace_back(CFG::Operation{ // input - std::visit(util::GenericVisitor{ - [&](FunctionCall const& _call) -> Stack { - CFG::Operation& operation = visitFunctionCall(_call); - yulAssert(assignedVariables.size() == operation.output.size(), ""); - return operation.output; - }, - [&](auto const& _identifierOrLiteral) -> Stack { - yulAssert(assignedVariables.size() == 1, ""); - return {(*this)(_identifierOrLiteral)}; - } - }, *_assignment.value), + visitAssignmentRightHandSide(*_assignment.value, assignedVariables.size()), // output assignedVariables | ranges::to, // operation @@ -264,35 +189,6 @@ void ControlFlowGraphBuilder::operator()(Block const& _block) std::visit(*this, statement); } -std::pair ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition) -{ - CFG::BasicBlock& nonZero = m_graph.makeBlock(); - CFG::BasicBlock& zero = m_graph.makeBlock(); - makeConditionalJump(move(_condition), nonZero, zero); - return {&nonZero, &zero}; -} - -void ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition, CFG::BasicBlock& _nonZero, CFG::BasicBlock& _zero) -{ - yulAssert(m_currentBlock, ""); - m_currentBlock->exit = CFG::BasicBlock::ConditionalJump{ - move(_condition), - &_nonZero, - &_zero - }; - _nonZero.entries.emplace_back(m_currentBlock); - _zero.entries.emplace_back(m_currentBlock); - m_currentBlock = nullptr; -} - -void ControlFlowGraphBuilder::jump(CFG::BasicBlock& _target, bool backwards) -{ - yulAssert(m_currentBlock, ""); - m_currentBlock->exit = CFG::BasicBlock::Jump{&_target, backwards}; - _target.entries.emplace_back(m_currentBlock); - m_currentBlock = &_target; -} - void ControlFlowGraphBuilder::operator()(If const& _if) { auto&& [ifBranch, afterIf] = makeConditionalJump(std::visit(*this, *_if.condition)); @@ -365,12 +261,7 @@ void ControlFlowGraphBuilder::operator()(ForLoop const& _loop) std::optional constantCondition; if (auto const* literalCondition = get_if(_loop.condition.get())) - { - if (valueOfLiteral(*literalCondition) == 0) - constantCondition = false; - else - constantCondition = true; - } + constantCondition = valueOfLiteral(*literalCondition) != 0; CFG::BasicBlock& loopCondition = m_graph.makeBlock(); CFG::BasicBlock& loopBody = m_graph.makeBlock(); @@ -465,10 +356,73 @@ void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function) builder.m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{&info}; } + +CFG::Operation& ControlFlowGraphBuilder::visitFunctionCall(FunctionCall const& _call) +{ + yulAssert(m_scope, ""); + yulAssert(m_currentBlock, ""); + + if (BuiltinFunction const* builtin = m_dialect.builtin(_call.functionName.name)) + { + Stack inputs; + for (auto&& [idx, arg]: _call.arguments | ranges::views::enumerate | ranges::views::reverse) + if (!builtin->literalArgument(idx).has_value()) + inputs.emplace_back(std::visit(*this, arg)); + CFG::BuiltinCall builtinCall{_call.debugData, *builtin, _call, inputs.size()}; + return m_currentBlock->operations.emplace_back(CFG::Operation{ + // input + std::move(inputs), + // output + ranges::views::iota(0u, builtin->returns.size()) | ranges::views::transform([&](size_t _i) { + return TemporarySlot{_call, _i}; + }) | ranges::to, + // operation + std::move(builtinCall) + }); + } + else + { + Scope::Function* function = nullptr; + yulAssert(m_scope->lookup(_call.functionName.name, util::GenericVisitor{ + [](Scope::Variable&) { yulAssert(false, "Expected function name."); }, + [&](Scope::Function& _function) { function = &_function; } + }), "Function name not found."); + yulAssert(function, ""); + Stack inputs{FunctionCallReturnLabelSlot{_call}}; + for (auto const& arg: _call.arguments | ranges::views::reverse) + inputs.emplace_back(std::visit(*this, arg)); + return m_currentBlock->operations.emplace_back(CFG::Operation{ + // input + std::move(inputs), + // output + ranges::views::iota(0u, function->returns.size()) | ranges::views::transform([&](size_t _i) { + return TemporarySlot{_call, _i}; + }) | ranges::to, + // operation + CFG::FunctionCall{_call.debugData, *function, _call} + }); + } +} + +Stack ControlFlowGraphBuilder::visitAssignmentRightHandSide(Expression const& _expression, size_t _expectedSlotCount) +{ + return std::visit(util::GenericVisitor{ + [&](FunctionCall const& _call) -> Stack { + CFG::Operation& operation = visitFunctionCall(_call); + yulAssert(_expectedSlotCount == operation.output.size(), ""); + return operation.output; + }, + [&](auto const& _identifierOrLiteral) -> Stack{ + yulAssert(_expectedSlotCount == 1, ""); + return {(*this)(_identifierOrLiteral)}; + } + }, _expression); +} + Scope::Variable const& ControlFlowGraphBuilder::lookupVariable(YulString _name) const { yulAssert(m_scope, ""); - Scope::Variable *var = nullptr; + Scope::Variable* var = nullptr; if (m_scope->lookup(_name, util::GenericVisitor{ [&](Scope::Variable& _var) { var = &_var; }, [](Scope::Function&) @@ -482,3 +436,32 @@ Scope::Variable const& ControlFlowGraphBuilder::lookupVariable(YulString _name) }; yulAssert(false, "External identifier access unimplemented."); } + +std::pair ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition) +{ + CFG::BasicBlock& nonZero = m_graph.makeBlock(); + CFG::BasicBlock& zero = m_graph.makeBlock(); + makeConditionalJump(move(_condition), nonZero, zero); + return {&nonZero, &zero}; +} + +void ControlFlowGraphBuilder::makeConditionalJump(StackSlot _condition, CFG::BasicBlock& _nonZero, CFG::BasicBlock& _zero) +{ + yulAssert(m_currentBlock, ""); + m_currentBlock->exit = CFG::BasicBlock::ConditionalJump{ + move(_condition), + &_nonZero, + &_zero + }; + _nonZero.entries.emplace_back(m_currentBlock); + _zero.entries.emplace_back(m_currentBlock); + m_currentBlock = nullptr; +} + +void ControlFlowGraphBuilder::jump(CFG::BasicBlock& _target, bool backwards) +{ + yulAssert(m_currentBlock, ""); + m_currentBlock->exit = CFG::BasicBlock::Jump{&_target, backwards}; + _target.entries.emplace_back(m_currentBlock); + m_currentBlock = &_target; +} diff --git a/libyul/backends/evm/ControlFlowGraphBuilder.h b/libyul/backends/evm/ControlFlowGraphBuilder.h index 7e969e7be..306c749bd 100644 --- a/libyul/backends/evm/ControlFlowGraphBuilder.h +++ b/libyul/backends/evm/ControlFlowGraphBuilder.h @@ -58,6 +58,7 @@ private: Dialect const& _dialect ); CFG::Operation& visitFunctionCall(FunctionCall const&); + Stack visitAssignmentRightHandSide(Expression const& _expression, size_t _expectedSlotCount); Scope::Variable const& lookupVariable(YulString _name) const; std::pair makeConditionalJump(StackSlot _condition); diff --git a/test/libyul/ControlFlowGraphTest.cpp b/test/libyul/ControlFlowGraphTest.cpp index 819df3c17..e691d159f 100644 --- a/test/libyul/ControlFlowGraphTest.cpp +++ b/test/libyul/ControlFlowGraphTest.cpp @@ -169,7 +169,7 @@ private: } size_t getBlockId(CFG::BasicBlock const& _block) { - if (size_t *id = util::valueOrNullptr(m_blockIds, &_block)) + if (size_t* id = util::valueOrNullptr(m_blockIds, &_block)) return *id; size_t id = m_blockIds[&_block] = m_blockCount++; m_blocksToPrint.emplace_back(&_block);