Skip to content

[Yul] Simplifier via broken #5147

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.

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

Merged
merged 3 commits into from
Oct 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion libyul/optimiser/ExpressionSimplifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <libyul/optimiser/SimplificationRules.h>
#include <libyul/optimiser/Semantics.h>
#include <libyul/optimiser/SSAValueTracker.h>

#include <libsolidity/inlineasm/AsmData.h>

Expand All @@ -36,13 +37,24 @@ using namespace dev::solidity;
void ExpressionSimplifier::visit(Expression& _expression)
{
ASTModifier::visit(_expression);
while (auto match = SimplificationRules::findFirstMatch(_expression))
while (auto match = SimplificationRules::findFirstMatch(_expression, m_ssaValues))
{
// Do not apply the rule if it removes non-constant parts of the expression.
// TODO: The check could actually be less strict than "movable".
// We only require "Does not cause side-effects".
// Note: References to variables that are only assigned once are always movable,
// so if the value of the variable is not movable, the expression that references
// the variable still is.

if (match->removesNonConstants && !MovableChecker(_expression).movable())
return;
_expression = match->action().toExpression(locationOf(_expression));
}
}

void ExpressionSimplifier::run(Block& _ast)
{
SSAValueTracker ssaValues;
ssaValues(_ast);
ExpressionSimplifier{ssaValues.values()}(_ast);
}
10 changes: 10 additions & 0 deletions libyul/optimiser/ExpressionSimplifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,24 @@ namespace yul

/**
* Applies simplification rules to all expressions.
* The component will work best if the code is in SSA form, but
* this is not required for correctness.
*
* Prerequisite: Disambiguator.
*/
class ExpressionSimplifier: public ASTModifier
{
public:
using ASTModifier::operator();
virtual void visit(Expression& _expression);

static void run(Block& _ast);
private:
explicit ExpressionSimplifier(std::map<std::string, Expression const*> _ssaValues):
m_ssaValues(std::move(_ssaValues))
{}

std::map<std::string, Expression const*> m_ssaValues;
};

}
Expand Down
53 changes: 53 additions & 0 deletions libyul/optimiser/SSAValueTracker.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
/**
* Component that collects variables that are never assigned to and their
* initial values.
*/

#include <libyul/optimiser/SSAValueTracker.h>

#include <libsolidity/inlineasm/AsmData.h>

using namespace std;
using namespace dev;
using namespace dev::yul;

void SSAValueTracker::operator()(Assignment const& _assignment)
{
for (auto const& var: _assignment.variableNames)
m_values.erase(var.name);
}

void SSAValueTracker::operator()(VariableDeclaration const& _varDecl)
{
if (_varDecl.variables.size() == 1)
setValue(_varDecl.variables.front().name, _varDecl.value.get());
else
for (auto const& var: _varDecl.variables)
setValue(var.name, nullptr);
}

void SSAValueTracker::setValue(string const& _name, Expression const* _value)
{
assertThrow(
m_values.count(_name) == 0,
OptimizerException,
"Source needs to be disambiguated."
);
m_values[_name] = _value;
}
58 changes: 58 additions & 0 deletions libyul/optimiser/SSAValueTracker.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
/**
* Component that collects variables that are never assigned to and their
* initial values.
*/

#pragma once

#include <libyul/optimiser/ASTWalker.h>

#include <string>
#include <map>
#include <set>

namespace dev
{
namespace yul
{

/**
* Class that walks the AST and stores the initial value of each variable
* that is never assigned to.
*
* Prerequisite: Disambiguator
*/
class SSAValueTracker: public ASTWalker
{
public:
using ASTWalker::operator();
virtual void operator()(VariableDeclaration const& _varDecl) override;
virtual void operator()(Assignment const& _assignment) override;

std::map<std::string, Expression const*> const& values() const { return m_values; }
Expression const* value(std::string const& _name) const { return m_values.at(_name); }

private:
void setValue(std::string const& _name, Expression const* _value);

std::map<std::string, Expression const*> m_values;
};

}
}
56 changes: 44 additions & 12 deletions libyul/optimiser/SimplificationRules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ using namespace dev;
using namespace dev::yul;


SimplificationRule<Pattern> const* SimplificationRules::findFirstMatch(Expression const& _expr)
SimplificationRule<Pattern> const* SimplificationRules::findFirstMatch(
Expression const& _expr,
map<string, Expression const*> const& _ssaValues
)
{
if (_expr.type() != typeid(FunctionalInstruction))
return nullptr;
Expand All @@ -46,7 +49,7 @@ SimplificationRule<Pattern> const* SimplificationRules::findFirstMatch(Expressio
for (auto const& rule: rules.m_rules[byte(instruction.instruction)])
{
rules.resetMatchGroups();
if (rule.pattern.matches(_expr))
if (rule.pattern.matches(_expr, _ssaValues))
return &rule;
}
return nullptr;
Expand Down Expand Up @@ -101,13 +104,25 @@ void Pattern::setMatchGroup(unsigned _group, map<unsigned, Expression const*>& _
m_matchGroups = &_matchGroups;
}

bool Pattern::matches(Expression const& _expr) const
bool Pattern::matches(Expression const& _expr, map<string, Expression const*> const& _ssaValues) const
{
Expression const* expr = &_expr;

// Resolve the variable if possible.
// Do not do it for "Any" because we can check identity better for variables.
if (m_kind != PatternKind::Any && _expr.type() == typeid(Identifier))
{
string const& varName = boost::get<Identifier>(_expr).name;
if (_ssaValues.count(varName))
expr = _ssaValues.at(varName);
}
assertThrow(expr, OptimizerException, "");

if (m_kind == PatternKind::Constant)
{
if (_expr.type() != typeid(Literal))
if (expr->type() != typeid(Literal))
return false;
Literal const& literal = boost::get<Literal>(_expr);
Literal const& literal = boost::get<Literal>(*expr);
if (literal.kind != assembly::LiteralKind::Number)
return false;
if (m_data && *m_data != u256(literal.value))
Expand All @@ -116,34 +131,51 @@ bool Pattern::matches(Expression const& _expr) const
}
else if (m_kind == PatternKind::Operation)
{
if (_expr.type() != typeid(FunctionalInstruction))
if (expr->type() != typeid(FunctionalInstruction))
return false;
FunctionalInstruction const& instr = boost::get<FunctionalInstruction>(_expr);
FunctionalInstruction const& instr = boost::get<FunctionalInstruction>(*expr);
if (m_instruction != instr.instruction)
return false;
assertThrow(m_arguments.size() == instr.arguments.size(), OptimizerException, "");
for (size_t i = 0; i < m_arguments.size(); ++i)
if (!m_arguments[i].matches(instr.arguments.at(i)))
if (!m_arguments[i].matches(instr.arguments.at(i), _ssaValues))
return false;
}
else
{
assertThrow(m_arguments.empty(), OptimizerException, "");
assertThrow(m_arguments.empty(), OptimizerException, "\"Any\" should not have arguments.");
}
// We support matching multiple expressions that require the same value
// based on identical ASTs, which have to be movable.

if (m_matchGroup)
{
// We support matching multiple expressions that require the same value
// based on identical ASTs, which have to be movable.

// TODO: add tests:
// - { let x := mload(0) let y := and(x, x) }
// - { let x := 4 let y := and(x, y) }

// This code uses `_expr` again for "Any", because we want the comparison to be done
// on the variables and not their values.
// The assumption is that CSE or local value numbering has been done prior to this step.

if (m_matchGroups->count(m_matchGroup))
{
assertThrow(m_kind == PatternKind::Any, OptimizerException, "Match group repetition for non-any.");
Expression const* firstMatch = (*m_matchGroups)[m_matchGroup];
assertThrow(firstMatch, OptimizerException, "Match set but to null.");
return
SyntacticalEqualityChecker::equal(*firstMatch, _expr) &&
MovableChecker(_expr).movable();
}
else
else if (m_kind == PatternKind::Any)
(*m_matchGroups)[m_matchGroup] = &_expr;
else
{
assertThrow(m_kind == PatternKind::Constant, OptimizerException, "Match group set for operation.");
// We do not use _expr here, because we want the actual number.
(*m_matchGroups)[m_matchGroup] = expr;
}
}
return true;
}
Expand Down
8 changes: 6 additions & 2 deletions libyul/optimiser/SimplificationRules.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ class SimplificationRules: public boost::noncopyable

/// @returns a pointer to the first matching pattern and sets the match
/// groups accordingly.
static SimplificationRule<Pattern> const* findFirstMatch(Expression const& _expr);
/// @param _ssaValues values of variables that are assigned exactly once.
static SimplificationRule<Pattern> const* findFirstMatch(
Expression const& _expr,
std::map<std::string, Expression const*> const& _ssaValues
);

/// Checks whether the rulelist is non-empty. This is usually enforced
/// by the constructor, but we had some issues with static initialization.
Expand Down Expand Up @@ -92,7 +96,7 @@ class Pattern
/// same expression equivalence class.
void setMatchGroup(unsigned _group, std::map<unsigned, Expression const*>& _matchGroups);
unsigned matchGroup() const { return m_matchGroup; }
bool matches(Expression const& _expr) const;
bool matches(Expression const& _expr, std::map<std::string, Expression const*> const& _ssaValues) const;

std::vector<Pattern> arguments() const { return m_arguments; }

Expand Down
14 changes: 13 additions & 1 deletion test/libyul/YulOptimizerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,19 @@ bool YulOptimizerTest::run(ostream& _stream, string const& _linePrefix, bool con
else if (m_optimizerStep == "expressionSimplifier")
{
disambiguate();
(ExpressionSimplifier{})(*m_ast);
ExpressionSimplifier::run(*m_ast);
}
else if (m_optimizerStep == "fullSimplify")
{
disambiguate();
NameDispenser nameDispenser;
nameDispenser.m_usedNames = NameCollector(*m_ast).names();
ExpressionSplitter{nameDispenser}(*m_ast);
CommonSubexpressionEliminator{}(*m_ast);
ExpressionSimplifier::run(*m_ast);
UnusedPruner::runUntilStabilised(*m_ast);
ExpressionJoiner::run(*m_ast);
ExpressionJoiner::run(*m_ast);
}
else if (m_optimizerStep == "unusedPruner")
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
let a := add(7, sub(mload(0), 7))
mstore(a, 0)
}
// ----
// fullSimplify
// {
// let _2 := 0
// mstore(mload(_2), _2)
// }
9 changes: 9 additions & 0 deletions test/libyul/yulOptimizerTests/fullSimplify/constants.yul
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
let a := add(1, mul(3, 4))
mstore(0, a)
}
// ----
// fullSimplify
// {
// mstore(0, 13)
// }
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
let a := sub(calldataload(0), calldataload(0))
mstore(a, 0)
}
// ----
// fullSimplify
// {
// mstore(0, 0)
// }
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
let a := sub(calldataload(1), calldataload(0))
mstore(0, a)
}
// ----
// fullSimplify
// {
// let _1 := 0
// mstore(_1, sub(calldataload(1), calldataload(_1)))
// }
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
let a := mload(0)
mstore(0, sub(a, a))
}
// ----
// fullSimplify
// {
// let _1 := 0
// pop(mload(_1))
// mstore(_1, 0)
// }
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
function f() -> a {}
let b := add(7, sub(f(), 7))
mstore(b, 0)
}
// ----
// fullSimplify
// {
// function f() -> a
// {
// }
// mstore(f(), 0)
// }
Loading