Skip to content

Commit b93deda

Browse files
blishkoblishko
committed
SSA-CFG: Store function call arguments in the same order as Yul
In SSA-CFG we were previously storing function call arguments in reversed order compared to Yul and EVM instructions. The reversed order is more natural for stack shuffler, but it can cause confusion in order places. We should use the same order as Yul and EVM. Since we now visit the arguments in different order, the order of operations in a basic block can change and this has effects down the line. For example, the stack layout might be different and eventually also the bytecode can differ.
1 parent 8471cf2 commit b93deda

33 files changed

Lines changed: 226 additions & 225 deletions

‎libyul/backends/evm/ssa/CodeTransform.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ void CodeTransform::operator()(InstId _instId, StackData const& _operationInputL
235235
yulAssert(m_stack.size() >= _inst.inputs.size());
236236
for (auto const& [stackEntry, input]: ranges::views::zip(
237237
m_stack | ranges::views::take_last(_inst.inputs.size()),
238-
_inst.inputs
238+
_inst.inputs | ranges::views::reverse
239239
))
240240
yulAssert(stackEntry.isValue() && stackEntry.value() == input);
241241

‎libyul/backends/evm/ssa/SSACFGBuilder.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ InstId SSACFGBuilder::visitFunctionCall(FunctionCall const& _call)
478478
literalArguments.emplace_back(std::get<Literal>(arg));
479479
}
480480
std::vector<InstId> inputs;
481-
for (auto&& [idx, arg]: _call.arguments | ranges::views::enumerate | ranges::views::reverse)
481+
for (auto&& [idx, arg]: _call.arguments | ranges::views::enumerate)
482482
if (!builtin.literalArgument(idx).has_value())
483483
inputs.emplace_back(std::visit(*this, arg));
484484
canContinue = builtin.controlFlowSideEffects.canContinue;
@@ -498,7 +498,7 @@ InstId SSACFGBuilder::visitFunctionCall(FunctionCall const& _call)
498498
yulAssert(calleeIt != m_functionRegistry.end(), "Called function has no registered graph id.");
499499
canContinue = m_sideEffects.functionSideEffects().at(calleeIt->second.definition).canContinue;
500500
std::vector<InstId> inputs;
501-
for (auto const& arg: _call.arguments | ranges::views::reverse)
501+
for (auto const& arg: _call.arguments)
502502
inputs.emplace_back(std::visit(*this, arg));
503503
return m_graph.makeCallWithProjections(
504504
m_currentBlock,

‎libyul/backends/evm/ssa/StackLayoutGenerator.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,9 @@ void StackLayoutGenerator::visitBlock(SSACFG::BlockId const& _blockId)
251251
requiredStackTop.emplace_back(Slot::makeFunctionCallReturnLabel(*callSiteID));
252252
}
253253
}
254-
requiredStackTop += _inst.inputs | ranges::views::transform([this](InstId const& _id) { return StackSlot::makeValue(m_cfg, _id); });
254+
requiredStackTop += _inst.inputs
255+
| ranges::views::reverse
256+
| ranges::views::transform([this](InstId const& _id) { return StackSlot::makeValue(m_cfg, _id); });
255257

256258
for (StackType::Depth depth{0}; depth < stack.size(); ++depth.value)
257259
if (

‎libyul/backends/evm/ssa/io/Printer.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
#include <fmt/format.h>
3131
#include <fmt/ranges.h>
3232

33-
#include <range/v3/view/reverse.hpp>
3433
#include <range/v3/view/transform.hpp>
3534
#include <range/v3/view/zip.hpp>
3635

@@ -74,7 +73,7 @@ void printBuiltinOperands(
7473
_out << ' ';
7574

7675
auto litIt = payload.literalArguments.begin();
77-
auto valIt = _inst.inputs.rbegin();
76+
auto valIt = _inst.inputs.begin();
7877
for (std::size_t i = 0; i < builtin.numParameters; ++i)
7978
{
8079
if (i > 0)
@@ -86,7 +85,7 @@ void printBuiltinOperands(
8685
}
8786
else
8887
{
89-
yulAssert(valIt != _inst.inputs.rend());
88+
yulAssert(valIt != _inst.inputs.end());
9089
_out << formatValueRef(*valIt++);
9190
}
9291
}
@@ -111,12 +110,12 @@ void printCallOperands(
111110

112111
_out << ' ';
113112
bool first = true;
114-
for (auto it = _inst.inputs.rbegin(); it != _inst.inputs.rend(); ++it)
113+
for (auto const input : _inst.inputs)
115114
{
116115
if (!first)
117116
_out << ", ";
118117
first = false;
119-
_out << formatValueRef(*it);
118+
_out << formatValueRef(input);
120119
}
121120
}
122121

‎test/libsolidity/semanticTests/externalContracts/FixedFeeRegistrar.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ contract FixedFeeRegistrar is Registrar {
8383
// gas legacy code: 792400
8484
// gas legacyOptimized: 84598
8585
// gas legacyOptimized code: 388000
86-
// gas ssaCFGOptimized: 78924
87-
// gas ssaCFGOptimized code: 322800
86+
// gas ssaCFGOptimized: 78694
87+
// gas ssaCFGOptimized code: 320000
8888
// reserve(string), 69 ether: 0x20, 3, "abc" ->
8989
// ~ emit Changed(string): #0x4e03657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45
9090
// gas irOptimized: 45741

‎test/libsolidity/semanticTests/externalContracts/base64.sol‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ contract test {
4141
// gas legacy code: 629800
4242
// gas legacyOptimized: 87926
4343
// gas legacyOptimized code: 429800
44-
// gas ssaCFGOptimized: 79463
45-
// gas ssaCFGOptimized code: 327800
44+
// gas ssaCFGOptimized: 79347
45+
// gas ssaCFGOptimized code: 326800
4646
// encode_inline_asm(bytes): 0x20, 0 -> 0x20, 0
4747
// encode_inline_asm(bytes): 0x20, 1, "f" -> 0x20, 4, "Zg=="
4848
// encode_inline_asm(bytes): 0x20, 2, "fo" -> 0x20, 4, "Zm8="
@@ -61,9 +61,9 @@ contract test {
6161
// gas irOptimized: 1406025
6262
// gas legacy: 1554038
6363
// gas legacyOptimized: 1132031
64-
// gas ssaCFGOptimized: 1388025
64+
// gas ssaCFGOptimized: 1397025
6565
// encode_no_asm_large()
6666
// gas irOptimized: 3512081
6767
// gas legacy: 4600082
6868
// gas legacyOptimized: 2813075
69-
// gas ssaCFGOptimized: 3077081
69+
// gas ssaCFGOptimized: 3044081

‎test/libsolidity/semanticTests/externalContracts/deposit_contract.sol‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ contract DepositContract is IDepositContract, ERC165 {
185185
// gas legacy code: 1438800
186186
// gas legacyOptimized: 848699
187187
// gas legacyOptimized code: 878200
188-
// gas ssaCFGOptimized: 809718
189-
// gas ssaCFGOptimized code: 570200
188+
// gas ssaCFGOptimized: 810061
189+
// gas ssaCFGOptimized code: 570400
190190
// supportsInterface(bytes4): 0x0 -> 0
191191
// supportsInterface(bytes4): 0xffffffff00000000000000000000000000000000000000000000000000000000 -> false # defined to be false by ERC-165 #
192192
// supportsInterface(bytes4): 0x01ffc9a700000000000000000000000000000000000000000000000000000000 -> true # ERC-165 id #
@@ -195,28 +195,28 @@ contract DepositContract is IDepositContract, ERC165 {
195195
// gas irOptimized: 109178
196196
// gas legacy: 142741
197197
// gas legacyOptimized: 117558
198-
// gas ssaCFGOptimized: 109652
198+
// gas ssaCFGOptimized: 109555
199199
// get_deposit_count() -> 0x20, 8, 0 # TODO: check balance and logs after each deposit #
200200
// deposit(bytes,bytes,bytes,bytes32), 32 ether: 0 -> FAILURE # Empty input #
201201
// get_deposit_root() -> 0xd70a234731285c6804c2a4f56711ddb8c82c99740f207854891028af34e27e5e
202202
// gas irOptimized: 109178
203203
// gas legacy: 142741
204204
// gas legacyOptimized: 117558
205-
// gas ssaCFGOptimized: 109652
205+
// gas ssaCFGOptimized: 109555
206206
// get_deposit_count() -> 0x20, 8, 0
207207
// deposit(bytes,bytes,bytes,bytes32), 1 ether: 0x80, 0xe0, 0x120, 0xaa4a8d0b7d9077248630f1a4701ae9764e42271d7f22b7838778411857fd349e, 0x30, 0x933ad9491b62059dd065b560d256d8957a8c402cc6e8d8ee7290ae11e8f73292, 0x67a8811c397529dac52ae1342ba58c9500000000000000000000000000000000, 0x20, 0x00f50428677c60f997aadeab24aabf7fceaef491c96a52b463ae91f95611cf71, 0x60, 0xa29d01cc8c6296a8150e515b5995390ef841dc18948aa3e79be6d7c1851b4cbb, 0x5d6ff49fa70b9c782399506a22a85193151b9b691245cebafd2063012443c132, 0x4b6c36debaedefb7b2d71b0503ffdc00150aaffd42e63358238ec888901738b8 -> # txhash: 0x7085c586686d666e8bb6e9477a0f0b09565b2060a11f1c4209d3a52295033832 #
208208
// ~ emit DepositEvent(bytes,bytes,bytes,bytes,bytes): 0xa0, 0x0100, 0x0140, 0x0180, 0x0200, 0x30, 0x933ad9491b62059dd065b560d256d8957a8c402cc6e8d8ee7290ae11e8f73292, 0x67a8811c397529dac52ae1342ba58c9500000000000000000000000000000000, 0x20, 0xf50428677c60f997aadeab24aabf7fceaef491c96a52b463ae91f95611cf71, 0x08, 0xca9a3b00000000000000000000000000000000000000000000000000000000, 0x60, 0xa29d01cc8c6296a8150e515b5995390ef841dc18948aa3e79be6d7c1851b4cbb, 0x5d6ff49fa70b9c782399506a22a85193151b9b691245cebafd2063012443c132, 0x4b6c36debaedefb7b2d71b0503ffdc00150aaffd42e63358238ec888901738b8, 0x08, 0x00
209209
// get_deposit_root() -> 0x2089653123d9c721215120b6db6738ba273bbc5228ac093b1f983badcdc8a438
210210
// gas irOptimized: 109174
211211
// gas legacy: 142750
212212
// gas legacyOptimized: 117570
213-
// gas ssaCFGOptimized: 109645
213+
// gas ssaCFGOptimized: 109548
214214
// get_deposit_count() -> 0x20, 8, 0x0100000000000000000000000000000000000000000000000000000000000000
215215
// deposit(bytes,bytes,bytes,bytes32), 32 ether: 0x80, 0xe0, 0x120, 0xdbd986dc85ceb382708cf90a3500f500f0a393c5ece76963ac3ed72eccd2c301, 0x30, 0xb2ce0f79f90e7b3a113ca5783c65756f96c4b4673c2b5c1eb4efc22280259441, 0x06d601211e8866dc5b50dc48a244dd7c00000000000000000000000000000000, 0x20, 0x00344b6c73f71b11c56aba0d01b7d8ad83559f209d0a4101a515f6ad54c89771, 0x60, 0x945caaf82d18e78c033927d51f452ebcd76524497b91d7a11219cb3db6a1d369, 0x7595fc095ce489e46b2ef129591f2f6d079be4faaf345a02c5eb133c072e7c56, 0x0c6c3617eee66b4b878165c502357d49485326bc6b31bc96873f308c8f19c09d -> # txhash: 0x404d8e109822ce448e68f45216c12cb051b784d068fbe98317ab8e50c58304ac #
216216
// ~ emit DepositEvent(bytes,bytes,bytes,bytes,bytes): 0xa0, 0x0100, 0x0140, 0x0180, 0x0200, 0x30, 0xb2ce0f79f90e7b3a113ca5783c65756f96c4b4673c2b5c1eb4efc22280259441, 0x06d601211e8866dc5b50dc48a244dd7c00000000000000000000000000000000, 0x20, 0x344b6c73f71b11c56aba0d01b7d8ad83559f209d0a4101a515f6ad54c89771, 0x08, 0x40597307000000000000000000000000000000000000000000000000000000, 0x60, 0x945caaf82d18e78c033927d51f452ebcd76524497b91d7a11219cb3db6a1d369, 0x7595fc095ce489e46b2ef129591f2f6d079be4faaf345a02c5eb133c072e7c56, 0x0c6c3617eee66b4b878165c502357d49485326bc6b31bc96873f308c8f19c09d, 0x08, 0x0100000000000000000000000000000000000000000000000000000000000000
217217
// get_deposit_root() -> 0x40255975859377d912c53aa853245ebd939bdd2b33a28e084babdcc1ed8238ee
218218
// gas irOptimized: 109174
219219
// gas legacy: 142750
220220
// gas legacyOptimized: 117570
221-
// gas ssaCFGOptimized: 109645
221+
// gas ssaCFGOptimized: 109548
222222
// get_deposit_count() -> 0x20, 8, 0x0200000000000000000000000000000000000000000000000000000000000000

‎test/libsolidity/semanticTests/externalContracts/prbmath_signed.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ contract test {
5757
// gas legacy code: 2205000
5858
// gas legacyOptimized: 178012
5959
// gas legacyOptimized code: 1669600
60-
// gas ssaCFGOptimized: 174817
61-
// gas ssaCFGOptimized code: 1636800
60+
// gas ssaCFGOptimized: 174865
61+
// gas ssaCFGOptimized code: 1637400
6262
// div(int256,int256): 3141592653589793238, 88714123 -> 35412542528203691288251815328
6363
// gas irOptimized: 22045
6464
// gas legacy: 22736

‎test/libsolidity/semanticTests/externalContracts/prbmath_unsigned.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ contract test {
5757
// gas legacy code: 1999000
5858
// gas legacyOptimized: 168857
5959
// gas legacyOptimized code: 1556200
60-
// gas ssaCFGOptimized: 167703
61-
// gas ssaCFGOptimized code: 1545800
60+
// gas ssaCFGOptimized: 167799
61+
// gas ssaCFGOptimized code: 1547000
6262
// div(uint256,uint256): 3141592653589793238, 88714123 -> 35412542528203691288251815328
6363
// gas irOptimized: 21912
6464
// gas legacy: 22475

‎test/libsolidity/semanticTests/externalContracts/ramanujan_pi.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ contract test {
4242
// gas legacy code: 523600
4343
// gas legacyOptimized: 82667
4444
// gas legacyOptimized code: 369200
45-
// gas ssaCFGOptimized: 78042
46-
// gas ssaCFGOptimized code: 314400
45+
// gas ssaCFGOptimized: 77994
46+
// gas ssaCFGOptimized code: 313800
4747
// prb_pi() -> 3141592656369545286
4848
// gas irOptimized: 55036
4949
// gas legacy: 100657

0 commit comments

Comments
 (0)