diff --git a/Changelog.md b/Changelog.md index 93d24cdaa..47df79890 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,7 @@ Language Features: Compiler Features: * General: Raise warning if runtime bytecode exceeds 24576 bytes (a limit introduced in Spurious Dragon). + * Yul Optimizer: Apply penalty when trying to rematerialize into loops. Bugfixes: diff --git a/libyul/optimiser/DataFlowAnalyzer.cpp b/libyul/optimiser/DataFlowAnalyzer.cpp index db1517f59..baa66553b 100644 --- a/libyul/optimiser/DataFlowAnalyzer.cpp +++ b/libyul/optimiser/DataFlowAnalyzer.cpp @@ -145,10 +145,14 @@ void DataFlowAnalyzer::operator()(FunctionDefinition& _fun) // Save all information. We might rather reinstantiate this class, // but this could be difficult if it is subclassed. map value; + map variableLoopDepth; + size_t loopDepth{0}; InvertibleRelation references; InvertibleMap storage; InvertibleMap memory; - m_value.swap(value); + swap(m_value, value); + swap(m_variableLoopDepth, variableLoopDepth); + swap(m_loopDepth, loopDepth); swap(m_references, references); swap(m_storage, storage); swap(m_memory, memory); @@ -168,7 +172,9 @@ void DataFlowAnalyzer::operator()(FunctionDefinition& _fun) // statement. popScope(); - m_value.swap(value); + swap(m_value, value); + swap(m_variableLoopDepth, variableLoopDepth); + swap(m_loopDepth, loopDepth); swap(m_references, references); swap(m_storage, storage); swap(m_memory, memory); @@ -180,6 +186,8 @@ void DataFlowAnalyzer::operator()(ForLoop& _for) // we would have to deal with more complicated scoping rules. assertThrow(_for.pre.statements.empty(), OptimizerException, ""); + ++m_loopDepth; + AssignmentsSinceContinue assignmentsSinceCont; assignmentsSinceCont(_for.body); @@ -202,6 +210,8 @@ void DataFlowAnalyzer::operator()(ForLoop& _for) clearKnowledgeIfInvalidated(*_for.condition); clearKnowledgeIfInvalidated(_for.post); clearKnowledgeIfInvalidated(_for.body); + + --m_loopDepth; } void DataFlowAnalyzer::operator()(Block& _block) @@ -222,7 +232,7 @@ void DataFlowAnalyzer::handleAssignment(set const& _variables, Expres movableChecker.visit(*_value); else for (auto const& var: _variables) - m_value[var] = &m_zero; + assignValue(var, &m_zero); if (_value && _variables.size() == 1) { @@ -230,7 +240,7 @@ void DataFlowAnalyzer::handleAssignment(set const& _variables, Expres // Expression has to be movable and cannot contain a reference // to the variable that will be assigned to. if (movableChecker.movable() && !movableChecker.referencedVariables().count(name)) - m_value[name] = _value; + assignValue(name, _value); } auto const& referencedVariables = movableChecker.referencedVariables(); @@ -296,11 +306,20 @@ void DataFlowAnalyzer::clearValues(set _variables) // Clear the value and update the reference relation. for (auto const& name: _variables) + { m_value.erase(name); + m_variableLoopDepth.erase(name); + } for (auto const& name: _variables) m_references.eraseKey(name); } +void DataFlowAnalyzer::assignValue(YulString _variable, Expression const* _value) +{ + m_value[_variable] = _value; + m_variableLoopDepth[_variable] = m_loopDepth; +} + void DataFlowAnalyzer::clearKnowledgeIfInvalidated(Block const& _block) { SideEffectsCollector sideEffects(m_dialect, _block, &m_functionSideEffects); diff --git a/libyul/optimiser/DataFlowAnalyzer.h b/libyul/optimiser/DataFlowAnalyzer.h index 45f41bea8..04de94197 100644 --- a/libyul/optimiser/DataFlowAnalyzer.h +++ b/libyul/optimiser/DataFlowAnalyzer.h @@ -110,6 +110,8 @@ protected: /// for example at points where control flow is merged. void clearValues(std::set _names); + void assignValue(YulString _variable, Expression const* _value); + /// Clears knowledge about storage or memory if they may be modified inside the block. void clearKnowledgeIfInvalidated(Block const& _block); @@ -144,6 +146,8 @@ protected: /// Current values of variables, always movable. std::map m_value; + /// The loop nesting depth of the definition of variables (those used in m_value). + std::map m_variableLoopDepth; /// m_references.forward[a].contains(b) <=> the current expression assigned to a references b /// m_references.backward[b].contains(a) <=> the current expression assigned to a references b InvertibleRelation m_references; @@ -153,6 +157,9 @@ protected: KnowledgeBase m_knowledgeBase; + /// Current nesting depth of loops. + size_t m_loopDepth{0}; + struct Scope { explicit Scope(bool _isFunction): isFunction(_isFunction) {} diff --git a/libyul/optimiser/Rematerialiser.cpp b/libyul/optimiser/Rematerialiser.cpp index 5d7350d97..02ac48e96 100644 --- a/libyul/optimiser/Rematerialiser.cpp +++ b/libyul/optimiser/Rematerialiser.cpp @@ -78,7 +78,12 @@ void Rematerialiser::visit(Expression& _e) auto const& value = *m_value.at(name); size_t refs = m_referenceCounts[name]; size_t cost = CodeCost::codeCost(m_dialect, value); - if (refs <= 1 || cost == 0 || (refs <= 5 && cost <= 1) || m_varsToAlwaysRematerialize.count(name)) + if ( + (refs <= 1 && m_variableLoopDepth.at(name) == m_loopDepth) || + cost == 0 || + (refs <= 5 && cost <= 1 && m_loopDepth == 0) || + m_varsToAlwaysRematerialize.count(name) + ) { assertThrow(m_referenceCounts[name] > 0, OptimizerException, ""); for (auto const& ref: m_references.forward[name]) diff --git a/libyul/optimiser/Rematerialiser.h b/libyul/optimiser/Rematerialiser.h index 2d70e0b0d..1316f0373 100644 --- a/libyul/optimiser/Rematerialiser.h +++ b/libyul/optimiser/Rematerialiser.h @@ -27,12 +27,13 @@ namespace solidity::yul { /** - * Optimisation stage that replaces variables by their most recently assigned expressions, + * Optimisation stage that replaces variable references by those expressions + * that are most recently assigned to the referenced variables, * but only if the expression is movable and one of the following holds: - * - the variable is referenced exactly once + * - the variable is referenced exactly once (and definition-to-reference does not cross a loop boundary) * - the value is extremely cheap ("cost" of zero like ``caller()``) * - the variable is referenced at most 5 times and the value is rather cheap - * ("cost" of at most 1 like a constant up to 0xff) + * ("cost" of at most 1 like a constant up to 0xff) and we are not in a loop * * Prerequisite: Disambiguator, ForLoopInitRewriter. */ @@ -68,6 +69,8 @@ protected: std::set _varsToAlwaysRematerialize = {} ); + using DataFlowAnalyzer::operator(); + using ASTModifier::visit; void visit(Expression& _e) override; diff --git a/test/libsolidity/gasTests/abiv2_optimised.sol b/test/libsolidity/gasTests/abiv2_optimised.sol index 92323fc68..b1bc1ff5d 100644 --- a/test/libsolidity/gasTests/abiv2_optimised.sol +++ b/test/libsolidity/gasTests/abiv2_optimised.sol @@ -17,9 +17,9 @@ contract C { // optimize-yul: true // ---- // creation: -// codeDepositCost: 605800 +// codeDepositCost: 603000 // executionCost: 638 -// totalCost: 606438 +// totalCost: 603638 // external: // a(): 1029 // b(uint256): 2084 diff --git a/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul b/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul index 0d37cf1b9..4557c1889 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/abi_example1.yul @@ -492,34 +492,37 @@ // } // function abi_decode_t_array$_t_array$_t_uint256_$2_memory_$dyn_memory_ptr(offset, end) -> array // { -// if iszero(slt(add(offset, 0x1f), end)) { revert(array, array) } +// let _1 := 0x1f +// if iszero(slt(add(offset, _1), end)) { revert(array, array) } // let length := calldataload(offset) // array := allocateMemory(array_allocation_size_t_array$_t_address_$dyn_memory(length)) // let dst := array // mstore(array, length) -// let _1 := 0x20 -// dst := add(array, _1) -// let src := add(offset, _1) -// if gt(add(add(offset, mul(length, 0x40)), _1), end) { revert(0, 0) } +// let _2 := 0x20 +// dst := add(array, _2) +// let src := add(offset, _2) +// let _3 := 0x40 +// if gt(add(add(offset, mul(length, _3)), _2), end) { revert(0, 0) } // let i := 0 // for { } lt(i, length) { i := add(i, 1) } // { -// if iszero(slt(add(src, 0x1f), end)) { revert(0, 0) } -// let dst_1 := allocateMemory(array_allocation_size_t_array$_t_uint256_$2_memory(0x2)) +// if iszero(slt(add(src, _1), end)) { revert(0, 0) } +// let _4 := 0x2 +// let dst_1 := allocateMemory(array_allocation_size_t_array$_t_uint256_$2_memory(_4)) // let dst_2 := dst_1 // let src_1 := src -// let _2 := add(src, 0x40) -// if gt(_2, end) { revert(0, 0) } +// let _5 := add(src, _3) +// if gt(_5, end) { revert(0, 0) } // let i_1 := 0 -// for { } lt(i_1, 0x2) { i_1 := add(i_1, 1) } +// for { } lt(i_1, _4) { i_1 := add(i_1, 1) } // { // mstore(dst_1, calldataload(src_1)) -// dst_1 := add(dst_1, _1) -// src_1 := add(src_1, _1) +// dst_1 := add(dst_1, _2) +// src_1 := add(src_1, _2) // } // mstore(dst, dst_2) -// dst := add(dst, _1) -// src := _2 +// dst := add(dst, _2) +// src := _5 // } // } // function abi_decode_t_array$_t_uint256_$dyn_memory_ptr(offset, end) -> array @@ -548,8 +551,9 @@ // for { } lt(i, 0x3) { i := add(i, 1) } // { // mstore(pos, and(mload(srcPtr), sub(shl(160, 1), 1))) -// srcPtr := add(srcPtr, 0x20) -// pos := add(pos, 0x20) +// let _1 := 0x20 +// srcPtr := add(srcPtr, _1) +// pos := add(pos, _1) // } // } // function allocateMemory(size) -> memPtr diff --git a/test/libyul/yulOptimizerTests/fullSuite/aztec.yul b/test/libyul/yulOptimizerTests/fullSuite/aztec.yul index 35e1bb7b3..635da72d6 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/aztec.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/aztec.yul @@ -375,7 +375,8 @@ // let i := 0 // for { } lt(i, n) { i := add(i, 0x01) } // { -// calldatacopy(add(0x300, mul(i, 0x80)), add(add(notes, mul(i, 0xc0)), 0x60), 0x80) +// let _1 := 0x80 +// calldatacopy(add(0x300, mul(i, _1)), add(add(notes, mul(i, 0xc0)), 0x60), _1) // } // mstore(0, keccak256(0x300, mul(n, 0x80))) // } diff --git a/test/libyul/yulOptimizerTests/fullSuite/clear_after_if_continue.yul b/test/libyul/yulOptimizerTests/fullSuite/clear_after_if_continue.yul index 19816b445..a9cdb84e0 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/clear_after_if_continue.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/clear_after_if_continue.yul @@ -12,7 +12,16 @@ // { // { // let y := mload(0x20) -// for { } and(y, 8) { if y { revert(0, 0) } } +// let _1 := iszero(and(y, 8)) +// for { } +// iszero(_1) +// { +// if y +// { +// let _2 := 0 +// revert(_2, _2) +// } +// } // { // if y { continue } // sstore(1, 0) diff --git a/test/libyul/yulOptimizerTests/fullSuite/devcon_example.yul b/test/libyul/yulOptimizerTests/fullSuite/devcon_example.yul index ff898b926..314f2a363 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/devcon_example.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/devcon_example.yul @@ -21,10 +21,12 @@ // { // let _1 := calldataload(0) // let sum := 0 +// let length := calldataload(_1) // let i := sum -// for { } lt(i, calldataload(_1)) { i := add(i, 1) } +// for { } lt(i, length) { i := add(i, 1) } // { -// sum := add(sum, calldataload(add(add(_1, mul(i, 0x20)), 0x20))) +// let _2 := 0x20 +// sum := add(sum, calldataload(add(add(_1, mul(i, _2)), _2))) // } // sstore(0, sum) // } diff --git a/test/libyul/yulOptimizerTests/fullSuite/loopInvariantCodeMotion.yul b/test/libyul/yulOptimizerTests/fullSuite/loopInvariantCodeMotion.yul index f570d2ecf..f8fb81aa8 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/loopInvariantCodeMotion.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/loopInvariantCodeMotion.yul @@ -24,10 +24,13 @@ // { // let _1 := calldataload(0) // let sum := 0 +// let length := calldataload(_1) // let i := sum -// for { } lt(i, calldataload(_1)) { i := add(i, 1) } +// let _2 := calldataload(7) +// for { } lt(i, length) { i := add(i, 1) } // { -// sum := add(sum, add(calldataload(add(add(_1, mul(i, 0x20)), 0x20)), calldataload(7))) +// let _3 := 0x20 +// sum := add(sum, add(calldataload(add(add(_1, mul(i, _3)), _3)), _2)) // } // sstore(0, sum) // } diff --git a/test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul b/test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul index 456f32d42..77c384021 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/no_move_loop_orig.yul @@ -12,10 +12,11 @@ // ---- // { // { +// let _1 := iszero(caller()) // for { } // 1 // { -// for { } caller() { } +// for { } iszero(_1) { } // { } // mstore(192, 0) // } diff --git a/test/libyul/yulOptimizerTests/rematerialiser/many_refs_small_cost_loop.yul b/test/libyul/yulOptimizerTests/rematerialiser/many_refs_small_cost_loop.yul new file mode 100644 index 000000000..0d42afa97 --- /dev/null +++ b/test/libyul/yulOptimizerTests/rematerialiser/many_refs_small_cost_loop.yul @@ -0,0 +1,28 @@ +{ + // Cost of rematerializating x is 1 + let x := 0xff + // Although x has low cost, it is not considered for + // rematerialization because it is referenced more than 5 times + for {} lt(x, 0x100) {} + { + let y := add(x, 1) + let z := mul(x, 1) + let a := div(x, 2) + let b := mod(x, 3) + let c := sdiv(x, 4) + } +} +// ==== +// step: rematerialiser +// ---- +// { +// let x := 0xff +// for { } lt(x, 0x100) { } +// { +// let y := add(x, 1) +// let z := mul(x, 1) +// let a := div(x, 2) +// let b := mod(x, 3) +// let c := sdiv(x, 4) +// } +// } diff --git a/test/libyul/yulOptimizerTests/rematerialiser/no_remat_in_loop.yul b/test/libyul/yulOptimizerTests/rematerialiser/no_remat_in_loop.yul new file mode 100644 index 000000000..8fdd1f002 --- /dev/null +++ b/test/libyul/yulOptimizerTests/rematerialiser/no_remat_in_loop.yul @@ -0,0 +1,37 @@ +{ + // origin has zero cost and thus will be rematerialised, + // calldataload(0) has low cost and will not be rematerialised + let a := origin() + let b := calldataload(0) + let i := 0 + let z := calldataload(9) + for {} lt(i, 10) {i := add(a, b)} { + // This will be rematerialised, because it stays inside + // the loop. + let x := calldataload(1) + mstore(9, x) + // No, because again one loop further down. + let y := calldataload(2) + for {} y {} { + // Again no. + mstore(12, z) + } + } +} +// ==== +// step: rematerialiser +// ---- +// { +// let a := origin() +// let b := calldataload(0) +// let i := 0 +// let z := calldataload(9) +// for { } lt(i, 10) { i := add(origin(), b) } +// { +// let x := calldataload(1) +// mstore(9, calldataload(1)) +// let y := calldataload(2) +// for { } y { } +// { mstore(12, z) } +// } +// } diff --git a/test/libyul/yulOptimizerTests/rematerialiser/some_refs_small_cost_loop.yul b/test/libyul/yulOptimizerTests/rematerialiser/some_refs_small_cost_loop.yul new file mode 100644 index 000000000..0eeb248a6 --- /dev/null +++ b/test/libyul/yulOptimizerTests/rematerialiser/some_refs_small_cost_loop.yul @@ -0,0 +1,17 @@ +{ + // Cost of rematerializating x is 1 + let x := 0xff + // Reference to x is not rematerialized because the reference is in a loop + for {} lt(x, 0x100) {} + { + let y := add(x, 1) + } +} +// ==== +// step: rematerialiser +// ---- +// { +// let x := 0xff +// for { } lt(x, 0x100) { } +// { let y := add(x, 1) } +// } diff --git a/test/libyul/yulOptimizerTests/rematerialiser/some_refs_small_cost_nested_loop.yul b/test/libyul/yulOptimizerTests/rematerialiser/some_refs_small_cost_nested_loop.yul new file mode 100644 index 000000000..c95cf5454 --- /dev/null +++ b/test/libyul/yulOptimizerTests/rematerialiser/some_refs_small_cost_nested_loop.yul @@ -0,0 +1,26 @@ +{ + // Cost of rematerializating x is 1 + let x := 0xff + // Although x has a low cost and fewer than 6 references, + // its references in a loop are not rematerialized + for {} lt(x, 0x100) {} + { + let y := add(x, 1) + for {} lt(x, 0x200) {} + { + let z := mul(x, 2) + } + } +} +// ==== +// step: rematerialiser +// ---- +// { +// let x := 0xff +// for { } lt(x, 0x100) { } +// { +// let y := add(x, 1) +// for { } lt(x, 0x200) { } +// { let z := mul(x, 2) } +// } +// }