Samsung / walrus

WebAssembly Lightweight RUntime
Apache License 2.0
35 stars 10 forks source link

[JIT] assertion failed in X64 JIT #202

Closed clover2123 closed 7 months ago

clover2123 commented 7 months ago

For x64 environment, lwnode with walrus-jit presents an error (assertion error) above.

clover2123 commented 7 months ago

Build walrus with the latest version of the main branch

zherczeg commented 7 months ago

May I get some more info for reproducing this bug? It does not shown here.

I am currently using these hashes: Walrus: 1d852c62188094b38ec060ed7b1d51ede07867fe (latest) Sljit: e09319736d051a248dffdf81257162ea331a2b4f (not the latest, but walrus uses it atm)

I confirmed with gdb that jit is compiled and used in lwnode. The project is compiled in debug mode.

The assertion line info is strange as well: https://github.com/zherczeg/sljit/blob/e09319736d051a248dffdf81257162ea331a2b4f/sljit_src/sljitLir.c#L2641

The sljit_has_cpu_feature(SLJIT_HAS_SIMD) should not fail unless you run it on a 15 year old machine (unlikely). Compiling the inline assembly with cpuid sometimes incorrect with some compilers, that can also be an issue.

zherczeg commented 7 months ago

I was able to reproduce! The code needs to be compiled in release mode. And it is cpuid. The "memory" keyword is missing from the assembly block, and it was added in a later version of the jit compiler:

https://github.com/zherczeg/sljit/commit/62c91605a8b20f74ab99902b338f626cc7551904

Perhaps we need a compiler update patch.

clover2123 commented 7 months ago

Many thanks for the quick update!

zherczeg commented 7 months ago

No problem. After the update it seems working now. It seems the Tensor test benchmarks itself. Based on that:

Interpreter: tf: 10.244s - 12.091s JIT: tf: 6.166s - 7.338s

Although the measured time varies, JIT visibly has some gain. The output is the same, so I hope it works.

clover2123 commented 7 months ago

It works well :) Fixed by #203