From 615438a890427f576bab5b7fcc5b3a8ec49098a1 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 11 Dec 2014 17:35:23 +0100 Subject: [PATCH] Swap literals to the end if optimizing. --- libsolidity/Compiler.cpp | 15 ++++++++++----- libsolidity/Compiler.h | 6 ++++-- libsolidity/CompilerStack.cpp | 4 ++-- libsolidity/ExpressionCompiler.cpp | 26 ++++++++++++++++++++------ libsolidity/ExpressionCompiler.h | 7 ++++--- libsolidity/Token.h | 2 ++ test/solidityOptimizerTest.cpp | 19 +++++++++++++++++-- 7 files changed, 59 insertions(+), 20 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index b094af194..d41bcffad 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -246,7 +246,7 @@ bool Compiler::visit(FunctionDefinition const& _function) bool Compiler::visit(IfStatement const& _ifStatement) { - ExpressionCompiler::compileExpression(m_context, _ifStatement.getCondition()); + compileExpression(_ifStatement.getCondition()); eth::AssemblyItem trueTag = m_context.appendConditionalJump(); if (_ifStatement.getFalseStatement()) _ifStatement.getFalseStatement()->accept(*this); @@ -265,7 +265,7 @@ bool Compiler::visit(WhileStatement const& _whileStatement) m_breakTags.push_back(loopEnd); m_context << loopStart; - ExpressionCompiler::compileExpression(m_context, _whileStatement.getCondition()); + compileExpression(_whileStatement.getCondition()); m_context << eth::Instruction::ISZERO; m_context.appendConditionalJumpTo(loopEnd); @@ -298,7 +298,7 @@ bool Compiler::visit(Return const& _return) //@todo modifications are needed to make this work with functions returning multiple values if (Expression const* expression = _return.getExpression()) { - ExpressionCompiler::compileExpression(m_context, *expression); + compileExpression(*expression); VariableDeclaration const& firstVariable = *_return.getFunctionReturnParameters().getParameters().front(); ExpressionCompiler::appendTypeConversion(m_context, *expression->getType(), *firstVariable.getType()); @@ -312,7 +312,7 @@ bool Compiler::visit(VariableDefinition const& _variableDefinition) { if (Expression const* expression = _variableDefinition.getExpression()) { - ExpressionCompiler::compileExpression(m_context, *expression); + compileExpression(*expression); ExpressionCompiler::appendTypeConversion(m_context, *expression->getType(), *_variableDefinition.getDeclaration().getType()); @@ -324,10 +324,15 @@ bool Compiler::visit(VariableDefinition const& _variableDefinition) bool Compiler::visit(ExpressionStatement const& _expressionStatement) { Expression const& expression = _expressionStatement.getExpression(); - ExpressionCompiler::compileExpression(m_context, expression); + compileExpression(expression); CompilerUtils(m_context).popStackElement(*expression.getType()); return false; } +void Compiler::compileExpression(Expression const& _expression) +{ + ExpressionCompiler::compileExpression(m_context, _expression, m_optimize); +} + } } diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index 4b8f02c5d..639e98410 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -30,10 +30,10 @@ namespace solidity { class Compiler: private ASTConstVisitor { public: - Compiler(): m_returnTag(m_context.newTag()) {} + explicit Compiler(bool _optimize = false): m_optimize(_optimize), m_returnTag(m_context.newTag()) {} void compileContract(ContractDefinition const& _contract, std::vector const& _magicGlobals); - bytes getAssembledBytecode(bool _optimize = false) { return m_context.getAssembledBytecode(_optimize); } + bytes getAssembledBytecode() { return m_context.getAssembledBytecode(m_optimize); } void streamAssembly(std::ostream& _stream) const { m_context.streamAssembly(_stream); } private: @@ -57,7 +57,9 @@ private: virtual bool visit(VariableDefinition const& _variableDefinition) override; virtual bool visit(ExpressionStatement const& _expressionStatement) override; + void compileExpression(Expression const& _expression); + bool const m_optimize; CompilerContext m_context; std::vector m_breakTags; ///< tag to jump to for a "break" statement std::vector m_continueTags; ///< tag to jump to for a "continue" statement diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index d46754856..7aaa79b1c 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -101,10 +101,10 @@ void CompilerStack::compile(bool _optimize) if (ContractDefinition* contract = dynamic_cast(node.get())) { m_globalContext->setCurrentContract(*contract); - shared_ptr compiler = make_shared(); + shared_ptr compiler = make_shared(_optimize); compiler->compileContract(*contract, m_globalContext->getMagicVariables()); Contract& compiledContract = m_contracts[contract->getName()]; - compiledContract.bytecode = compiler->getAssembledBytecode(_optimize); + compiledContract.bytecode = compiler->getAssembledBytecode(); compiledContract.compiler = move(compiler); } } diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index a0b0a54a8..a19c9cfd8 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -33,9 +33,9 @@ using namespace std; namespace dev { namespace solidity { -void ExpressionCompiler::compileExpression(CompilerContext& _context, Expression const& _expression) +void ExpressionCompiler::compileExpression(CompilerContext& _context, Expression const& _expression, bool _optimize) { - ExpressionCompiler compiler(_context); + ExpressionCompiler compiler(_context, _optimize); _expression.accept(compiler); } @@ -145,10 +145,24 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation) if (Token::isCompareOp(op) || op == Token::DIV || op == Token::MOD) cleanupNeeded = true; - rightExpression.accept(*this); - appendTypeConversion(*rightExpression.getType(), commonType, cleanupNeeded); - leftExpression.accept(*this); - appendTypeConversion(*leftExpression.getType(), commonType, cleanupNeeded); + // for commutative operators, push the literal as late as possible to allow improved optimization + //@todo this has to be extended for literal expressions + bool swap = (m_optimize && Token::isCommutativeOp(op) && dynamic_cast(&rightExpression) + && !dynamic_cast(&leftExpression)); + if (swap) + { + leftExpression.accept(*this); + appendTypeConversion(*leftExpression.getType(), commonType, cleanupNeeded); + rightExpression.accept(*this); + appendTypeConversion(*rightExpression.getType(), commonType, cleanupNeeded); + } + else + { + rightExpression.accept(*this); + appendTypeConversion(*rightExpression.getType(), commonType, cleanupNeeded); + leftExpression.accept(*this); + appendTypeConversion(*leftExpression.getType(), commonType, cleanupNeeded); + } if (Token::isCompareOp(op)) appendCompareOperatorCode(op, commonType); else diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index 41cb66763..fb4577c8c 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -45,14 +45,14 @@ class ExpressionCompiler: private ASTConstVisitor { public: /// Compile the given @a _expression into the @a _context. - static void compileExpression(CompilerContext& _context, Expression const& _expression); + static void compileExpression(CompilerContext& _context, Expression const& _expression, bool _optimize = false); /// Appends code to remove dirty higher order bits in case of an implicit promotion to a wider type. static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack, Type const& _targetType); private: - ExpressionCompiler(CompilerContext& _compilerContext): - m_context(_compilerContext), m_currentLValue(m_context) {} + explicit ExpressionCompiler(CompilerContext& _compilerContext, bool _optimize = false): + m_optimize(_optimize), m_context(_compilerContext), m_currentLValue(m_context) {} virtual bool visit(Assignment const& _assignment) override; virtual void endVisit(UnaryOperation const& _unaryOperation) override; @@ -132,6 +132,7 @@ private: unsigned m_stackSize; }; + bool m_optimize; CompilerContext& m_context; LValue m_currentLValue; }; diff --git a/libsolidity/Token.h b/libsolidity/Token.h index f1a94af35..c748d9892 100644 --- a/libsolidity/Token.h +++ b/libsolidity/Token.h @@ -317,6 +317,8 @@ public: static bool isElementaryTypeName(Value tok) { return INT <= tok && tok < TYPES_END; } static bool isAssignmentOp(Value tok) { return ASSIGN <= tok && tok <= ASSIGN_MOD; } static bool isBinaryOp(Value op) { return COMMA <= op && op <= MOD; } + static bool isCommutativeOp(Value op) { return op == BIT_OR || op == BIT_XOR || op == BIT_AND || + op == ADD || op == MUL || op == EQ || op == NE; } static bool isArithmeticOp(Value op) { return ADD <= op && op <= MOD; } static bool isCompareOp(Value op) { return EQ <= op && op <= IN; } diff --git a/test/solidityOptimizerTest.cpp b/test/solidityOptimizerTest.cpp index f8d3b552c..69000a57f 100644 --- a/test/solidityOptimizerTest.cpp +++ b/test/solidityOptimizerTest.cpp @@ -48,7 +48,7 @@ public: m_optimize = true; bytes optimizedBytecode = compileAndRun(_sourceCode, _value, _contractName); int sizeDiff = nonOptimizedBytecode.size() - optimizedBytecode.size(); - BOOST_CHECK_MESSAGE(sizeDiff >= _expectedSizeDecrease, "Bytecode did only shrink by " + BOOST_CHECK_MESSAGE(sizeDiff == _expectedSizeDecrease, "Bytecode did only shrink by " + boost::lexical_cast(sizeDiff) + " bytes, expected: " + boost::lexical_cast(_expectedSizeDecrease)); m_optimizedContract = m_contractAddress; @@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(invariants) return (((a + (1 - 1)) ^ 0) | 0) & (uint(0) - 1); } })"; - compileBothVersions(19, sourceCode); + compileBothVersions(28, sourceCode); compareVersions(0, u256(0x12334664)); } @@ -124,6 +124,21 @@ BOOST_AUTO_TEST_CASE(unused_expressions) compareVersions(0); } +BOOST_AUTO_TEST_CASE(constant_folding_both_sides) +{ + // if constants involving the same associative and commutative operator are applied from both + // sides, the operator should be applied only once, because the expression compiler + // (even in non-optimized mode) pushes literals as late as possible + char const* sourceCode = R"( + contract test { + function f(uint x) returns (uint y) { + return 98 ^ (7 * ((1 | (x | 1000)) * 40) ^ 102); + } + })"; + compileBothVersions(31, sourceCode); + compareVersions(0); +} + BOOST_AUTO_TEST_SUITE_END() }