diff --git a/Changelog.md b/Changelog.md index bacc170d7..575e040d6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,9 @@ ### 0.5.8 (unreleased) +Important Bugfixes: + * Yul Optimizer: Fix SSA transform for multi-assignments. + + Language Features: * Code Generation: Implement copying recursive structs from storage to memory. diff --git a/libyul/optimiser/SSATransform.cpp b/libyul/optimiser/SSATransform.cpp index 0c38e8d82..625d82bdc 100644 --- a/libyul/optimiser/SSATransform.cpp +++ b/libyul/optimiser/SSATransform.cpp @@ -62,17 +62,13 @@ void SSATransform::operator()(Block& _block) { set variablesToClearAtEnd; - // Creates a new variable (and returns its declaration) with value _value - // and replaces _value by a reference to that new variable. - - auto replaceByNew = [&](SourceLocation _loc, YulString _varName, YulString _type, unique_ptr& _value) -> VariableDeclaration + // Creates a new variable and stores it in the current variable value map. + auto newVariable = [&](YulString _varName) -> YulString { YulString newName = m_nameDispenser.newName(_varName); m_currentVariableValues[_varName] = newName; variablesToClearAtEnd.emplace(_varName); - unique_ptr v = make_unique(Identifier{_loc, newName}); - _value.swap(v); - return VariableDeclaration{_loc, {TypedName{_loc, std::move(newName), std::move(_type)}}, std::move(v)}; + return newName; }; iterateReplacing( @@ -84,36 +80,60 @@ void SSATransform::operator()(Block& _block) VariableDeclaration& varDecl = boost::get(_s); if (varDecl.value) visit(*varDecl.value); - if (varDecl.variables.size() != 1 || !m_variablesToReplace.count(varDecl.variables.front().name)) + + bool needToReplaceSome = false; + for (auto const& var: varDecl.variables) + if (m_variablesToReplace.count(var.name)) + needToReplaceSome = true; + if (!needToReplaceSome) return {}; - vector v; + // Replace "let a := v" by "let a_1 := v let a := a_1" - v.emplace_back(replaceByNew( - varDecl.location, - varDecl.variables.front().name, - varDecl.variables.front().type, - varDecl.value - )); - v.emplace_back(move(varDecl)); - return std::move(v); + // Replace "let a, b := v" by "let a_1, b_1 := v let a := a_1 let b := b_2" + auto loc = varDecl.location; + vector statements; + statements.emplace_back(VariableDeclaration{loc, {}, std::move(varDecl.value)}); + TypedNameList newVariables; + for (auto const& var: varDecl.variables) + { + YulString newName = newVariable(var.name); + YulString oldName = var.name; + newVariables.emplace_back(TypedName{loc, newName, {}}); + statements.emplace_back(VariableDeclaration{ + loc, + {TypedName{loc, oldName, {}}}, + make_unique(Identifier{loc, newName}) + }); + } + boost::get(statements.front()).variables = std::move(newVariables); + return std::move(statements); } else if (_s.type() == typeid(Assignment)) { Assignment& assignment = boost::get(_s); visit(*assignment.value); - if (assignment.variableNames.size() != 1) - return {}; - assertThrow(m_variablesToReplace.count(assignment.variableNames.front().name), OptimizerException, ""); - vector v; + for (auto const& var: assignment.variableNames) + assertThrow(m_variablesToReplace.count(var.name), OptimizerException, ""); + // Replace "a := v" by "let a_1 := v a := v" - v.emplace_back(replaceByNew( - assignment.location, - assignment.variableNames.front().name, - {}, // TODO determine type - assignment.value - )); - v.emplace_back(move(assignment)); - return std::move(v); + // Replace "a, b := v" by "let a_1, b_1 := v a := a_1 b := b_2" + auto loc = assignment.location; + vector statements; + statements.emplace_back(VariableDeclaration{loc, {}, std::move(assignment.value)}); + TypedNameList newVariables; + for (auto const& var: assignment.variableNames) + { + YulString newName = newVariable(var.name); + YulString oldName = var.name; + newVariables.emplace_back(TypedName{loc, newName, {}}); + statements.emplace_back(Assignment{ + loc, + {Identifier{loc, oldName}}, + make_unique(Identifier{loc, newName}) + }); + } + boost::get(statements.front()).variables = std::move(newVariables); + return std::move(statements); } else visit(_s); diff --git a/libyul/optimiser/SSATransform.h b/libyul/optimiser/SSATransform.h index 4cb62f238..1a367afc7 100644 --- a/libyul/optimiser/SSATransform.h +++ b/libyul/optimiser/SSATransform.h @@ -87,6 +87,8 @@ private: { } NameDispenser& m_nameDispenser; + /// This is a set of all variables that are assigned to anywhere in the code. + /// Variables that are only declared but never re-assigned are not touched. std::set const& m_variablesToReplace; std::map m_currentVariableValues; }; diff --git a/test/libyul/yulOptimizerTests/fullSuite/ssaReverse.yul b/test/libyul/yulOptimizerTests/fullSuite/ssaReverse.yul index ceaa7ceed..3e79e4aa1 100644 --- a/test/libyul/yulOptimizerTests/fullSuite/ssaReverse.yul +++ b/test/libyul/yulOptimizerTests/fullSuite/ssaReverse.yul @@ -36,13 +36,13 @@ // ---- // { // let a, b := abi_decode_t_bytes_calldata_ptr(mload(0), mload(1)) -// a, b := abi_decode_t_bytes_calldata_ptr(a, b) -// a, b := abi_decode_t_bytes_calldata_ptr(a, b) -// a, b := abi_decode_t_bytes_calldata_ptr(a, b) -// a, b := abi_decode_t_bytes_calldata_ptr(a, b) -// a, b := abi_decode_t_bytes_calldata_ptr(a, b) -// a, b := abi_decode_t_bytes_calldata_ptr(a, b) -// mstore(a, b) +// let a_1, b_1 := abi_decode_t_bytes_calldata_ptr(a, b) +// let a_2, b_2 := abi_decode_t_bytes_calldata_ptr(a_1, b_1) +// let a_3, b_3 := abi_decode_t_bytes_calldata_ptr(a_2, b_2) +// let a_4, b_4 := abi_decode_t_bytes_calldata_ptr(a_3, b_3) +// let a_5, b_5 := abi_decode_t_bytes_calldata_ptr(a_4, b_4) +// let a_6, b_6 := abi_decode_t_bytes_calldata_ptr(a_5, b_5) +// mstore(a_6, b_6) // function abi_decode_t_bytes_calldata_ptr(offset, end) -> arrayPos, length // { // if iszero(slt(add(offset, 0x1f), end)) diff --git a/test/libyul/yulOptimizerTests/ssaTransform/multi_assign.yul b/test/libyul/yulOptimizerTests/ssaTransform/multi_assign.yul new file mode 100644 index 000000000..41905cc79 --- /dev/null +++ b/test/libyul/yulOptimizerTests/ssaTransform/multi_assign.yul @@ -0,0 +1,29 @@ +{ + let a := mload(0) + let b := mload(1) + a, b := f() + sstore(a, b) + a := mload(5) + b := mload(a) + function f() -> x, y {} +} +// ==== +// step: ssaTransform +// ---- +// { +// let a_1 := mload(0) +// let a := a_1 +// let b_2 := mload(1) +// let b := b_2 +// let a_3, b_4 := f() +// a := a_3 +// b := b_4 +// sstore(a_3, b_4) +// let a_5 := mload(5) +// a := a_5 +// let b_6 := mload(a_5) +// b := b_6 +// function f() -> x, y +// { +// } +// } diff --git a/test/libyul/yulOptimizerTests/ssaTransform/multi_decl.yul b/test/libyul/yulOptimizerTests/ssaTransform/multi_decl.yul new file mode 100644 index 000000000..fea7403f6 --- /dev/null +++ b/test/libyul/yulOptimizerTests/ssaTransform/multi_decl.yul @@ -0,0 +1,25 @@ +{ + let x, y := f(1, 2) + x := mload(y) + y := mload(x) + let a, b := f(x, y) + sstore(a, b) + function f(t, v) -> x, y {} +} +// ==== +// step: ssaTransform +// ---- +// { +// let x_2, y_3 := f(1, 2) +// let x := x_2 +// let y := y_3 +// let x_4 := mload(y_3) +// x := x_4 +// let y_5 := mload(x_4) +// y := y_5 +// let a, b := f(x_4, y_5) +// sstore(a, b) +// function f(t, v) -> x_1, y_2 +// { +// } +// }