LibWasm: Switch to wasm-tools for test gen and update to latest spectest #7014

alimpfard · 2025-12-04T00:04:33Z

No description provided.

ladybird-bot · 2025-12-04T00:04:57Z

Hello!

One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

Hendiadyoin1

First glance from my end, will likely take another look at somepoint

AK/DoublyLinkedList.h

AK/Vector.h

Libraries/LibWasm/Parser/Parser.cpp

Libraries/LibWasm/Printer/Printer.cpp

Hendiadyoin1 · 2025-12-04T10:30:36Z

Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp

+    Vector<Value, ArgumentsStaticSize> args = move(arguments);
+    return configuration.call(interpreter, address, args);


so this is now explicitly not reusing the allocation, and previously was, or am I missing something

Can't pass a reference with different inline capacities, I think the move ctor just yoinks the allocation if its big enough anyway.

Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp

Hendiadyoin1 · 2025-12-04T10:40:36Z

Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp

@@ -105,12 +105,18 @@ constexpr static u32 default_sources_and_destination = (to_underlying(Dispatch::
 template<u64 opcode>
 struct InstructionHandler { };

+struct __attribute__((packed)) ShortenedIPAndAddresses {


you sure the packed is needed?

as it would be quite odd for a simple type to be sized like u32 and still have u64 like alignment

or does that affect the calling convention
(mainly making sure as packing can be a footgun when accidentally handing out references)

GCC was splitting the struct up and undoing the fix without it, I got annoyed and just slapped packed there to make it stop

a union would likely also work, but that probably not really nicer

alimpfard · 2025-12-05T14:43:43Z

Hmm, looks like this regressed a bunch of benchmarks, drafting until I figure that out.

WasmCoremark    coremark-minimal.wasm       0.797  695.448 ± 692.259 … 698.147  554.250 ± 553.738 … 555.219
WasmMicroBench  call-00-args.wasm           0.809  2.724 ± 2.722 … 2.726        3.367 ± 3.311 … 3.432
WasmMicroBench  call-01-args.wasm           1.065  6.175 ± 6.138 … 6.204        5.797 ± 5.548 … 6.187
WasmMicroBench  call-02-args.wasm           1.081  6.292 ± 6.254 … 6.365        5.819 ± 5.783 … 5.847
WasmMicroBench  call-03-args.wasm           1.054  6.487 ± 6.464 … 6.515        6.157 ± 6.083 … 6.281
WasmMicroBench  call-04-args.wasm           1.046  6.822 ± 6.776 … 6.865        6.524 ± 6.425 … 6.628
WasmMicroBench  call-16-args.wasm           1.069  11.907 ± 11.831 … 12.044     11.141 ± 11.058 … 11.305
WasmMicroBench  call-32-args.wasm           0.94   16.817 ± 16.776 … 16.895     17.882 ± 17.866 … 17.905
WasmRustBench   base64-bench.wasm           0.783  17.661 ± 17.553 … 17.826     22.558 ± 22.488 … 22.671
WasmRustBench   regex-match-bench.wasm      0.778  3.620 ± 3.594 … 3.660        4.653 ± 4.634 … 4.664
WasmRustBench   sha512-bench.wasm           0.498  24.170 ± 24.066 … 24.360     48.580 ± 48.376 … 48.695
WasmCoremark    Total                       0.797  695.448                      554.250
WasmMicroBench  Total                       1.009  57.222                       56.687
WasmRustBench   Total                       0.6    45.451                       75.790
All Suites      Total                       0.778  120.115                      154.354

Hendiadyoin1 · 2025-12-05T14:47:40Z

AK/DoublyLinkedList.h

        static_assert(
-            requires { T(value); }, "Conversion operator is missing.");
-        auto* node = make_node(forward<U>(value));
+            requires { T(forward<Args>(args)...); }, "Initializer is missing.");


Same here I guess

also technically an empend now
and kinda weird that the non-try append doesnt have the check relying on the one here

that's...true, will "fix".

Hendiadyoin1 · 2025-12-05T15:14:43Z

Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp

@@ -105,12 +105,18 @@ constexpr static u32 default_sources_and_destination = (to_underlying(Dispatch::
 template<u64 opcode>
 struct InstructionHandler { };

+struct __attribute__((packed)) ShortenedIPAndAddresses {


a union would likely also work, but that probably not really nicer

alimpfard · 2025-12-07T14:08:05Z

I think I'll make this PR only about the test generation and fixes, the performance stuff will have to stay in the oven a bit more.

We'll be using wasm-tools for Wasm testsuite generation in the next few commits.

Previously we were reading the arguments in an incorrect order, and placing the result in the wrong slot. This also removes the hacky implementation of accumulative relaxed dot, and just implements it directly as a new operator.

The instruction would be rejected for _much_ smaller values, but we shouldn't try to calculate (u64)1<<x with x>64.

alimpfard force-pushed the wasm-fix-lots branch 3 times, most recently from 2cc0c70 to 74fccfb Compare December 4, 2025 02:58

Hendiadyoin1 reviewed Dec 4, 2025

View reviewed changes

alimpfard force-pushed the wasm-fix-lots branch 11 times, most recently from 5eb10d8 to 40a8d9e Compare December 4, 2025 20:41

alimpfard marked this pull request as draft December 5, 2025 14:43

Hendiadyoin1 reviewed Dec 5, 2025

View reviewed changes

alimpfard added 6 commits December 7, 2025 15:08

LibWasm: Better handle oversized table allocation requests

d0eda1c

LibWeb: Make errors for missing wasm import errors slightly nicer

20cc8a7

Meta: Pull wasm-tools in CI instead of wabt

2d1d369

We'll be using wasm-tools for Wasm testsuite generation in the next few commits.

Meta: Generate Wasm tests using wasm-tools instead of wabt

c8101e6

LibWasm: Ensure alignment values larger than 64 are rejected

50156ad

The instruction would be rejected for _much_ smaller values, but we shouldn't try to calculate (u64)1<<x with x>64.

alimpfard force-pushed the wasm-fix-lots branch from 9c13a44 to 50156ad Compare December 7, 2025 14:10

alimpfard changed the title ~~LibWasm: Switch to wasm-tools for test gen and update to latest spectest (plus random performance fixes)~~ LibWasm: Switch to wasm-tools for test gen and update to latest spectest Dec 7, 2025

alimpfard marked this pull request as ready for review December 7, 2025 14:11

		Vector<Value, ArgumentsStaticSize> args = move(arguments);
		return configuration.call(interpreter, address, args);

Uh oh!

LibWasm: Switch to wasm-tools for test gen and update to latest spectest #7014

Are you sure you want to change the base?

LibWasm: Switch to wasm-tools for test gen and update to latest spectest #7014

Uh oh!

Conversation

alimpfard commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ladybird-bot commented Dec 4, 2025

Uh oh!

Hendiadyoin1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alimpfard commented Dec 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alimpfard commented Dec 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alimpfard commented Dec 4, 2025 •

edited

Loading

Hendiadyoin1 left a comment •

edited

Loading