diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 92d574fb1..258336917 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -109,8 +109,8 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) callDataUnpackerEntryPoints.push_back(m_context.newTag()); m_context << eth::dupInstruction(2) << eth::dupInstruction(2) << eth::Instruction::EQ; m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.back()); - m_context << eth::dupInstruction(4) << eth::Instruction::ADD; - //@todo avoid the last ADD (or remove it in the optimizer) + if (funid < interfaceFunctions.size() - 1) + m_context << eth::dupInstruction(4) << eth::Instruction::ADD; } m_context << eth::Instruction::STOP; // function not found @@ -241,7 +241,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); @@ -260,7 +260,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); @@ -293,7 +293,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()); @@ -307,7 +307,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()); @@ -319,10 +319,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 8408882ac..3beb423de 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 32822248f..c0b173d46 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -46,14 +46,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; @@ -133,6 +133,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 21b74eceb..51450f1a2 100644 --- a/libsolidity/Token.h +++ b/libsolidity/Token.h @@ -350,6 +350,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/solidityCompiler.cpp b/test/solidityCompiler.cpp index 004740b5e..eae8f3142 100644 --- a/test/solidityCompiler.cpp +++ b/test/solidityCompiler.cpp @@ -87,7 +87,7 @@ BOOST_AUTO_TEST_CASE(smoke_test) "}\n"; bytes code = compileContract(sourceCode); - unsigned boilerplateSize = 42; + unsigned boilerplateSize = 40; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x0, // initialize local variable x byte(Instruction::PUSH1), 0x2, @@ -107,8 +107,8 @@ BOOST_AUTO_TEST_CASE(different_argument_numbers) "}\n"; bytes code = compileContract(sourceCode); - unsigned shift = 70; - unsigned boilerplateSize = 83; + unsigned shift = 68; + unsigned boilerplateSize = 81; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x0, // initialize return variable d byte(Instruction::DUP3), @@ -158,8 +158,8 @@ BOOST_AUTO_TEST_CASE(ifStatement) "}\n"; bytes code = compileContract(sourceCode); - unsigned shift = 29; - unsigned boilerplateSize = 42; + unsigned shift = 27; + unsigned boilerplateSize = 40; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x0, byte(Instruction::DUP1), @@ -200,8 +200,8 @@ BOOST_AUTO_TEST_CASE(loops) "}\n"; bytes code = compileContract(sourceCode); - unsigned shift = 29; - unsigned boilerplateSize = 42; + unsigned shift = 27; + unsigned boilerplateSize = 40; bytes expectation({byte(Instruction::JUMPDEST), byte(Instruction::JUMPDEST), byte(Instruction::PUSH1), 0x1, diff --git a/test/solidityOptimizerTest.cpp b/test/solidityOptimizerTest.cpp index b689fe54e..388e0579c 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 >= (int)_expectedSizeDecrease, "Bytecode did only shrink by " + BOOST_CHECK_MESSAGE(sizeDiff == int(_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() }