Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use side-effects of user defined functions in evm code transform. #12132

Merged
merged 2 commits into from
Nov 22, 2022
Merged

Use side-effects of user defined functions in evm code transform. #12132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ba8c10
Use side-effects of user defined functions in evm code transform.
chriseth Oct 13, 2021
1b6063e
Disable zeppelin test failing due to hardhat heuristics.
ekpyron Mar 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Compiler Features:
* Commandline Interface: Add `--no-cbor-metadata` that skips CBOR metadata from getting appended at the end of the bytecode.
* Natspec: Add event Natspec inheritance for devdoc.
* Standard JSON: Add a boolean field `settings.metadata.appendCBOR` that skips CBOR metadata from getting appended at the end of the bytecode.
* Yul EVM Code Transform: Generate more optimal code for user-defined functions that always terminate a transaction. No return labels will be pushed for calls to functions that always terminate.
* Yul Optimizer: Allow replacing the previously hard-coded cleanup sequence by specifying custom steps after a colon delimiter (``:``) in the sequence string.
* Language Server: Add basic document hover support.
* Optimizer: Added optimization rule ``and(shl(X, Y), shl(X, Z)) => shl(X, and(Y, Z))``.
Expand Down
4 changes: 4 additions & 0 deletions libyul/backends/evm/ControlFlowGraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ struct CFG
/// True, if the call is recursive, i.e. entering the function involves a control flow path (potentially involving
/// more intermediate function calls) that leads back to this very call.
bool recursive = false;
/// True, if the call can return.
bool canContinue = true;
};
struct Assignment
{
Expand Down Expand Up @@ -210,10 +212,12 @@ struct CFG
{
std::shared_ptr<DebugData const> debugData;
Scope::Function const& function;
FunctionDefinition const& functionDefinition;
BasicBlock* entry = nullptr;
std::vector<VariableSlot> parameters;
std::vector<VariableSlot> returnVariables;
std::vector<BasicBlock*> exits;
bool canContinue = true;
ekpyron marked this conversation as resolved.
Show resolved Hide resolved
};

/// The main entry point, i.e. the start of the outermost Yul block.
Expand Down
76 changes: 43 additions & 33 deletions libyul/backends/evm/ControlFlowGraphBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <libyul/AST.h>
#include <libyul/Exceptions.h>
#include <libyul/Utilities.h>
#include <libyul/ControlFlowSideEffectsCollector.h>

#include <libsolutil/cxx20.h>
#include <libsolutil/Visitor.h>
Expand Down Expand Up @@ -214,7 +215,8 @@ std::unique_ptr<CFG> ControlFlowGraphBuilder::build(
auto result = std::make_unique<CFG>();
result->entry = &result->makeBlock(debugDataOf(_block));

ControlFlowGraphBuilder builder(*result, _analysisInfo, _dialect);
ControlFlowSideEffectsCollector sideEffects(_dialect, _block);
ControlFlowGraphBuilder builder(*result, _analysisInfo, sideEffects.functionSideEffects(), _dialect);
builder.m_currentBlock = result->entry;
builder(_block);

Expand All @@ -232,10 +234,12 @@ std::unique_ptr<CFG> ControlFlowGraphBuilder::build(
ControlFlowGraphBuilder::ControlFlowGraphBuilder(
CFG& _graph,
AsmAnalysisInfo const& _analysisInfo,
map<FunctionDefinition const*, ControlFlowSideEffects> const& _functionSideEffects,
Dialect const& _dialect
):
m_graph(_graph),
m_info(_analysisInfo),
m_functionSideEffects(_functionSideEffects),
m_dialect(_dialect)
{
}
Expand Down Expand Up @@ -285,10 +289,10 @@ void ControlFlowGraphBuilder::operator()(Assignment const& _assignment)
return VariableSlot{lookupVariable(_var.name), _var.debugData};
}) | ranges::to<vector<VariableSlot>>;

yulAssert(m_currentBlock, "");
Stack input = visitAssignmentRightHandSide(*_assignment.value, assignedVariables.size());
yulAssert(m_currentBlock);
m_currentBlock->operations.emplace_back(CFG::Operation{
// input
visitAssignmentRightHandSide(*_assignment.value, assignedVariables.size()),
std::move(input),
// output
assignedVariables | ranges::to<Stack>,
// operation
Expand All @@ -297,24 +301,13 @@ void ControlFlowGraphBuilder::operator()(Assignment const& _assignment)
}
void ControlFlowGraphBuilder::operator()(ExpressionStatement const& _exprStmt)
{
yulAssert(m_currentBlock, "");
std::visit(util::GenericVisitor{
[&](FunctionCall const& _call) {
Stack const& output = visitFunctionCall(_call);
yulAssert(output.empty(), "");
},
[&](auto const&) { yulAssert(false, ""); }
}, _exprStmt.expression);

// TODO: Ideally this would be done on the expression label and for all functions that always revert,
// not only for builtins.
if (auto const* funCall = get_if<FunctionCall>(&_exprStmt.expression))
if (BuiltinFunction const* builtin = m_dialect.builtin(funCall->functionName.name))
if (builtin->controlFlowSideEffects.terminatesOrReverts())
{
m_currentBlock->exit = CFG::BasicBlock::Terminated{};
m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock));
}
}

void ControlFlowGraphBuilder::operator()(Block const& _block)
Expand All @@ -331,7 +324,8 @@ void ControlFlowGraphBuilder::operator()(If const& _if)
{
auto& ifBranch = m_graph.makeBlock(debugDataOf(_if.body));
auto& afterIf = m_graph.makeBlock(debugDataOf(*m_currentBlock));
makeConditionalJump(debugDataOf(_if), std::visit(*this, *_if.condition), ifBranch, afterIf);
StackSlot condition = std::visit(*this, *_if.condition);
makeConditionalJump(debugDataOf(_if), std::move(condition), ifBranch, afterIf);
m_currentBlock = &ifBranch;
(*this)(_if.body);
jump(debugDataOf(_if.body), afterIf);
Expand All @@ -349,8 +343,9 @@ void ControlFlowGraphBuilder::operator()(Switch const& _switch)
// Artificially generate:
// let <ghostVariable> := <switchExpression>
VariableSlot ghostVarSlot{ghostVar, debugDataOf(*_switch.expression)};
StackSlot expression = std::visit(*this, *_switch.expression);
m_currentBlock->operations.emplace_back(CFG::Operation{
Stack{std::visit(*this, *_switch.expression)},
Stack{std::move(expression)},
Stack{ghostVarSlot},
CFG::Assignment{_switch.debugData, {ghostVarSlot}}
});
Expand Down Expand Up @@ -430,7 +425,8 @@ void ControlFlowGraphBuilder::operator()(ForLoop const& _loop)
else
{
jump(debugDataOf(_loop.pre), loopCondition);
makeConditionalJump(debugDataOf(*_loop.condition), std::visit(*this, *_loop.condition), loopBody, afterLoop);
StackSlot condition = std::visit(*this, *_loop.condition);
makeConditionalJump(debugDataOf(*_loop.condition), std::move(condition), loopBody, afterLoop);
m_currentBlock = &loopBody;
(*this)(_loop.body);
jump(debugDataOf(_loop.body), post);
Expand Down Expand Up @@ -473,41 +469,43 @@ void ControlFlowGraphBuilder::operator()(FunctionDefinition const& _function)

CFG::FunctionInfo& functionInfo = m_graph.functionInfo.at(&function);

ControlFlowGraphBuilder builder{m_graph, m_info, m_dialect};
ControlFlowGraphBuilder builder{m_graph, m_info, m_functionSideEffects, m_dialect};
builder.m_currentFunction = &functionInfo;
builder.m_currentBlock = functionInfo.entry;
builder(_function.body);
functionInfo.exits.emplace_back(builder.m_currentBlock);
builder.m_currentBlock->exit = CFG::BasicBlock::FunctionReturn{debugDataOf(_function), &functionInfo};
}

void ControlFlowGraphBuilder::registerFunction(FunctionDefinition const& _function)
void ControlFlowGraphBuilder::registerFunction(FunctionDefinition const& _functionDefinition)
{
yulAssert(m_scope, "");
yulAssert(m_scope->identifiers.count(_function.name), "");
Scope::Function& function = std::get<Scope::Function>(m_scope->identifiers.at(_function.name));
yulAssert(m_scope->identifiers.count(_functionDefinition.name), "");
Scope::Function& function = std::get<Scope::Function>(m_scope->identifiers.at(_functionDefinition.name));

yulAssert(m_info.scopes.at(&_function.body), "");
Scope* virtualFunctionScope = m_info.scopes.at(m_info.virtualBlocks.at(&_function).get()).get();
yulAssert(m_info.scopes.at(&_functionDefinition.body), "");
Scope* virtualFunctionScope = m_info.scopes.at(m_info.virtualBlocks.at(&_functionDefinition).get()).get();
yulAssert(virtualFunctionScope, "");

bool inserted = m_graph.functionInfo.emplace(std::make_pair(&function, CFG::FunctionInfo{
_function.debugData,
_functionDefinition.debugData,
function,
&m_graph.makeBlock(debugDataOf(_function.body)),
_function.parameters | ranges::views::transform([&](auto const& _param) {
_functionDefinition,
&m_graph.makeBlock(debugDataOf(_functionDefinition.body)),
_functionDefinition.parameters | ranges::views::transform([&](auto const& _param) {
return VariableSlot{
std::get<Scope::Variable>(virtualFunctionScope->identifiers.at(_param.name)),
_param.debugData
};
}) | ranges::to<vector>,
_function.returnVariables | ranges::views::transform([&](auto const& _retVar) {
_functionDefinition.returnVariables | ranges::views::transform([&](auto const& _retVar) {
return VariableSlot{
std::get<Scope::Variable>(virtualFunctionScope->identifiers.at(_retVar.name)),
_retVar.debugData
};
}) | ranges::to<vector>,
{}
{},
m_functionSideEffects.at(&_functionDefinition).canContinue
})).second;
yulAssert(inserted);
}
Expand All @@ -517,14 +515,16 @@ Stack const& ControlFlowGraphBuilder::visitFunctionCall(FunctionCall const& _cal
yulAssert(m_scope, "");
yulAssert(m_currentBlock, "");

Stack const* output = nullptr;
bool canContinue = true;
aarlt marked this conversation as resolved.
Show resolved Hide resolved
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{
output = &m_currentBlock->operations.emplace_back(CFG::Operation{
// input
std::move(inputs),
// output
Expand All @@ -534,24 +534,34 @@ Stack const& ControlFlowGraphBuilder::visitFunctionCall(FunctionCall const& _cal
// operation
std::move(builtinCall)
}).output;
canContinue = builtin->controlFlowSideEffects.canContinue;
}
else
{
Scope::Function const& function = lookupFunction(_call.functionName.name);
Stack inputs{FunctionCallReturnLabelSlot{_call}};
canContinue = m_graph.functionInfo.at(&function).canContinue;
Stack inputs;
if (canContinue)
inputs.emplace_back(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{
output = &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<Stack>,
// operation
CFG::FunctionCall{_call.debugData, function, _call}
CFG::FunctionCall{_call.debugData, function, _call, /* recursive */ false, canContinue}
}).output;
}
if (!canContinue)
{
m_currentBlock->exit = CFG::BasicBlock::Terminated{};
m_currentBlock = &m_graph.makeBlock(debugDataOf(*m_currentBlock));
cameel marked this conversation as resolved.
Show resolved Hide resolved
}
return *output;
}

Stack ControlFlowGraphBuilder::visitAssignmentRightHandSide(Expression const& _expression, size_t _expectedSlotCount)
Expand Down
3 changes: 3 additions & 0 deletions libyul/backends/evm/ControlFlowGraphBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#pragma once

#include <libyul/backends/evm/ControlFlowGraph.h>
#include <libyul/ControlFlowSideEffects.h>

namespace solidity::yul
{
Expand Down Expand Up @@ -55,6 +56,7 @@ class ControlFlowGraphBuilder
ControlFlowGraphBuilder(
CFG& _graph,
AsmAnalysisInfo const& _analysisInfo,
std::map<FunctionDefinition const*, ControlFlowSideEffects> const& _functionSideEffects,
Dialect const& _dialect
);
void registerFunction(FunctionDefinition const& _function);
Expand All @@ -77,6 +79,7 @@ class ControlFlowGraphBuilder
);
CFG& m_graph;
AsmAnalysisInfo const& m_info;
std::map<FunctionDefinition const*, ControlFlowSideEffects> const& m_functionSideEffects;
Dialect const& m_dialect;
CFG::BasicBlock* m_currentBlock = nullptr;
Scope* m_scope = nullptr;
Expand Down
40 changes: 24 additions & 16 deletions libyul/backends/evm/OptimizedEVMCodeTransform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,35 +71,39 @@ void OptimizedEVMCodeTransform::operator()(CFG::FunctionCall const& _call)
// Validate stack.
{
yulAssert(m_assembly.stackHeight() == static_cast<int>(m_stack.size()), "");
yulAssert(m_stack.size() >= _call.function.get().arguments.size() + 1, "");
yulAssert(m_stack.size() >= _call.function.get().arguments.size() + (_call.canContinue ? 1 : 0), "");
// Assert that we got the correct arguments on stack for the call.
for (auto&& [arg, slot]: ranges::zip_view(
_call.functionCall.get().arguments | ranges::views::reverse,
m_stack | ranges::views::take_last(_call.functionCall.get().arguments.size())
))
validateSlot(slot, arg);
// Assert that we got the correct return label on stack.
auto const* returnLabelSlot = get_if<FunctionCallReturnLabelSlot>(
&m_stack.at(m_stack.size() - _call.functionCall.get().arguments.size() - 1)
);
yulAssert(returnLabelSlot && &returnLabelSlot->call.get() == &_call.functionCall.get(), "");
if (_call.canContinue)
{
auto const* returnLabelSlot = get_if<FunctionCallReturnLabelSlot>(
&m_stack.at(m_stack.size() - _call.functionCall.get().arguments.size() - 1)
);
yulAssert(returnLabelSlot && &returnLabelSlot->call.get() == &_call.functionCall.get(), "");
}
cameel marked this conversation as resolved.
Show resolved Hide resolved
}

// Emit code.
{
m_assembly.setSourceLocation(originLocationOf(_call));
m_assembly.appendJumpTo(
getFunctionLabel(_call.function),
static_cast<int>(_call.function.get().returns.size() - _call.function.get().arguments.size()) - 1,
static_cast<int>(_call.function.get().returns.size() - _call.function.get().arguments.size()) - (_call.canContinue ? 1 : 0),
AbstractAssembly::JumpType::IntoFunction
);
m_assembly.appendLabel(m_returnLabels.at(&_call.functionCall.get()));
if (_call.canContinue)
m_assembly.appendLabel(m_returnLabels.at(&_call.functionCall.get()));
}

// Update stack.
{
// Remove arguments and return label from m_stack.
for (size_t i = 0; i < _call.function.get().arguments.size() + 1; ++i)
ekpyron marked this conversation as resolved.
Show resolved Hide resolved
for (size_t i = 0; i < _call.function.get().arguments.size() + (_call.canContinue ? 1 : 0); ++i)
m_stack.pop_back();
// Push return values to m_stack.
for (size_t index: ranges::views::iota(0u, _call.function.get().returns.size()))
Expand Down Expand Up @@ -479,8 +483,9 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block)
},
[&](CFG::BasicBlock::FunctionReturn const& _functionReturn)
{
yulAssert(m_currentFunctionInfo, "");
yulAssert(m_currentFunctionInfo == _functionReturn.info, "");
yulAssert(m_currentFunctionInfo);
yulAssert(m_currentFunctionInfo == _functionReturn.info);
yulAssert(m_currentFunctionInfo->canContinue);

// Construct the function return layout, which is fully determined by the function signature.
Stack exitStack = m_currentFunctionInfo->returnVariables | ranges::views::transform([](auto const& _varSlot){
Expand All @@ -494,11 +499,13 @@ void OptimizedEVMCodeTransform::operator()(CFG::BasicBlock const& _block)
},
[&](CFG::BasicBlock::Terminated const&)
{
// Assert that the last builtin call was in fact terminating.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - if we wanted to, we could of course drag along the entire logic into the code transform as well and keep validating things here - but I guess it's fine to trust that it's ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added at least some validation back, even though it relies on functionCall->canContinue to be correct. But conceptually that's fine - the canContinue flag is part of the CFG and the code transform only guarantees correctness relative to the CFG...

yulAssert(!_block.operations.empty(), "");
CFG::BuiltinCall const* builtinCall = get_if<CFG::BuiltinCall>(&_block.operations.back().operation);
yulAssert(builtinCall, "");
yulAssert(builtinCall->builtin.get().controlFlowSideEffects.terminatesOrReverts(), "");
yulAssert(!_block.operations.empty());
if (CFG::BuiltinCall const* builtinCall = get_if<CFG::BuiltinCall>(&_block.operations.back().operation))
yulAssert(builtinCall->builtin.get().controlFlowSideEffects.terminatesOrReverts(), "");
else if (CFG::FunctionCall const* functionCall = get_if<CFG::FunctionCall>(&_block.operations.back().operation))
yulAssert(!functionCall->canContinue);
else
yulAssert(false);
}
}, _block.exit);
// TODO: We could assert that the last emitted assembly item terminated or was an (unconditional) jump.
Expand All @@ -515,7 +522,8 @@ void OptimizedEVMCodeTransform::operator()(CFG::FunctionInfo const& _functionInf
yulAssert(m_stack.empty() && m_assembly.stackHeight() == 0, "");

// Create function entry layout in m_stack.
m_stack.emplace_back(FunctionReturnLabelSlot{_functionInfo.function});
if (_functionInfo.canContinue)
m_stack.emplace_back(FunctionReturnLabelSlot{_functionInfo.function});
for (auto const& param: _functionInfo.parameters | ranges::views::reverse)
m_stack.emplace_back(param);
m_assembly.setStackHeight(static_cast<int>(m_stack.size()));
Expand Down
Loading