Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add op-assign instructions to wasmi IR #794

Closed
wants to merge 17 commits into from

Conversation

Robbepop
Copy link
Member

@Robbepop Robbepop commented Nov 23, 2023

Closes #767.

TODO

  • Define new op-assign instructions in wasmi IR.
  • Implement execution of new op-assign instructions.
  • Implement translation of op-assign instructions.
  • Implement optimizations from or to op-assign instructions.
    • local.set: op-assign -> op
    • local.set: op -> op-assign
    • cmp-assign+branch -> fuse
  • Add tests to cover new functionality.

This commit still misses:
- docs for the new instructions
- execution of the new instructions
- translation of the new instructions
- optimization involving the new instructions
@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Nov 23, 2023

BENCHMARKS

NATIVEWASMTIME
BENCHMARKMASTERPRDIFFMASTERPRDIFFWASMTIME OVERHEAD
execute/
bare_call_0
1.55ms 1.65ms 🔴 6.95% 1.18ms 1.16ms 🔴 -1.96% 🟢 -30%
execute/
bare_call_0/typed
1.17ms 1.17ms ⚪ -0.84% 801.20µs 928.34µs 🔴 15.87% 🟢 -20%
execute/
bare_call_1
1.62ms 1.70ms 🔴 5.19% 1.28ms 1.31ms 🔴 2.46% 🟢 -23%
execute/
bare_call_16
2.52ms 2.52ms 🔴 -0.18% 3.55ms 3.56ms 🔴 0.59% 🟢 41%
execute/
bare_call_16/typed
1.55ms 1.63ms 🔴 5.01% 1.95ms 2.00ms 🔴 2.47% 🟢 23%
execute/
bare_call_1/typed
1.27ms 1.29ms 🔴 1.80% 998.11µs 996.96µs ⚪ -0.07% 🟢 -23%
execute/
bare_call_4
1.78ms 1.78ms 🔴 0.00% 1.71ms 1.63ms 🔴 -4.75% 🟢 -8%
execute/
bare_call_4/typed
1.22ms 1.32ms 🔴 8.69% 1.05ms 1.11ms 🔴 5.64% 🟢 -16%
execute/
br_table
1.39ms 1.37ms ⚪ -1.07% 1.14ms 1.18ms 🔴 3.54% 🟢 -14%
execute/
count_until
546.17µs 545.74µs ⚪ -0.10% 1.69ms 1.68ms ⚪ -0.79% 🔴 207%
execute/
factorial_iterative
322.11µs 320.17µs ⚪ -0.61% 805.50µs 805.50µs ⚪ 0.06% 🔴 152%
execute/
factorial_recursive
490.12µs 493.30µs ⚪ 0.62% 969.61µs 969.54µs ⚪ -0.02% 🟡 97%
execute/
fibonacci_iter
1.40ms 1.40ms ⚪ 0.34% 3.81ms 3.84ms ⚪ 0.36% 🔴 174%
execute/
fibonacci_rec
4.04ms 4.00ms ⚪ -1.09% 8.59ms 8.56ms ⚪ -0.35% 🔴 114%
execute/
fibonacci_tail
859.45µs 864.14µs ⚪ 0.57% 2.20ms 2.17ms ⚪ -1.08% 🔴 151%
execute/
global_bump
746.86µs 741.03µs ⚪ -0.78% 2.19ms 2.20ms ⚪ 0.46% 🔴 197%
execute/
global_const
659.02µs 660.22µs ⚪ 0.08% 2.44ms 2.42ms ⚪ -0.60% 🔴 267%
execute/
host_calls
37.17µs 36.05µs 🟢 -3.19% 39.87µs 40.38µs 🔴 1.23% 🟢 12%
execute/
memory_fill
1.15ms 1.21ms 🔴 5.55% 3.32ms 3.30ms ⚪ -0.61% 🔴 172%
execute/
memory_sum
1.14ms 1.18ms ⚪ 2.43% 3.29ms 3.30ms ⚪ 0.26% 🔴 179%
execute/
memory_vec_add
2.34ms 2.39ms ⚪ 1.85% 7.41ms 7.39ms ⚪ -0.22% 🔴 209%
execute/
recursive_is_even
663.42µs 662.37µs ⚪ -0.14% 1.47ms 1.45ms 🟢 -1.18% 🔴 119%
execute/
recursive_ok
94.34µs 94.01µs ⚪ -0.47% 200.90µs 200.71µs ⚪ -0.07% 🔴 114%
execute/
recursive_scan
129.19µs 129.42µs ⚪ 0.38% 284.87µs 287.38µs ⚪ 0.88% 🔴 122%
execute/
recursive_trap
8.80µs 8.90µs 🔴 1.16% 20.96µs 20.94µs ⚪ -0.05% 🔴 135%
execute/
regex_redux
457.04µs 463.29µs 🔴 1.45% 1.23ms 1.24ms ⚪ 0.40% 🔴 167%
execute/
rev_complement
424.09µs 422.19µs ⚪ -0.46% 1.14ms 1.17ms 🔴 2.34% 🔴 176%
execute/
tiny_keccak
317.45µs 323.09µs 🔴 1.73% 1.09ms 1.10ms ⚪ 0.42% 🔴 240%
execute/
trunc_f2i
737.07µs 732.38µs ⚪ -0.56% 1.71ms 1.71ms ⚪ 0.09% 🔴 134%
instantiate/
wasm_kernel
54.94µs 55.79µs 🔴 3.46% 56.22µs 58.93µs 🔴 2.72% 🟢 6%
translate/
erc1155
210.40µs 208.07µs ⚪ -1.07% 385.12µs 364.78µs 🟢 -5.40% 🟡 75%
translate/
erc20
103.72µs 103.67µs ⚪ -0.09% 182.12µs 178.39µs 🟢 -2.04% 🟡 72%
translate/
erc721
145.99µs 146.34µs ⚪ 0.08% 263.76µs 257.33µs 🟢 -2.53% 🟡 76%
translate/
spidermonkey
64.01ms 64.04ms ⚪ -0.22% 0.00ns 0.00ns 🟢 -2.08% 🟢 -100%
translate/
wasm_kernel
4.17ms 4.18ms ⚪ 0.30% 6.79ms 6.61ms 🟢 -2.88% 🟡 58%

Link to pipeline

@Robbepop
Copy link
Member Author

The performance improvements are extremely modest given that this PR is adding well over 100 new wasmi IR instructions. So either we are doing something wrong or this whole effort is definitely not worth it.

@Robbepop
Copy link
Member Author

Closed since deemed not to be worth the efforts performance wise.

@Robbepop Robbepop closed this Nov 24, 2023
Robbepop added a commit that referenced this pull request Nov 24, 2023
This work was carried over from #794.
@Robbepop Robbepop deleted the rf-add-op-assign-instrs branch March 11, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add op-assign instructions.
2 participants