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.

Sign up for GitHub

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

Jump to bottom

wasm2c/spec/simd_address.txt fails on non-x86 #2240

Open
rathann opened this issue May 25, 2023 · 7 comments · May be fixed by #2545
Open

wasm2c/spec/simd_address.txt fails on non-x86 #2240

rathann opened this issue May 25, 2023 · 7 comments · May be fixed by #2545

Comments

@rathann
Copy link
Contributor

rathann commented May 25, 2023

When running the test suite of wabt-1.0.33 on aarch64 on Fedora 38, I get this (new) test failure:

- test/wasm2c/spec/simd_address.txt
  expected error code 0, got 1.
  STDERR MISMATCH:
  --- expected
  +++ actual
  @@ -0,0 +1,11 @@
  +Traceback (most recent call last):
  +  File "/builddir/build/BUILD/wabt-1.0.33/test/run-spec-wasm2c.py", line 648, in <module>
  +    sys.exit(main(sys.argv[1:]))
  +             ^^^^^^^^^^^^^^^^^^
  +  File "/builddir/build/BUILD/wabt-1.0.33/test/run-spec-wasm2c.py", line 642, in main
  +    utils.Executable(main_exe, forward_stdout=True).RunWithArgs()
  +  File "/builddir/build/BUILD/wabt-1.0.33/test/utils.py", line 95, in RunWithArgs
  +    raise error
  +utils.Error: Error running "out/test/wasm2c/spec/simd_address/simd_address" (1):
  +None
  +out/test/wasm2c/spec/simd_address/simd_address-main.c:499: assertion failed: expected w2c_simd__address__1__wasm_v1280x2Eload_offset_65521(&simd__address__1__wasm_instance) to trap.
  STDOUT MISMATCH:
  --- expected
  +++ actual
  @@ -1 +1 @@
  -42/42 tests passed.
  +41/42 tests passed.
@keithw
Copy link
Member

keithw commented May 25, 2023 •
edited
Loading

Thanks as always for these reports! I think this particular one is expected and "disclosed" here: https://github.com/WebAssembly/wabt/blob/main/src/template/wasm2c_simd.declarations.c#L6

The root issue here is that:

  • out-of-bounds accesses on Wasm must trap deterministically, but
  • wasm2c tries to use the MMU to detect out-of-bounds accesses and raise a signal, but this kind of side-effect isn't something the compiler cares to preserve if the load can otherwise be optizmed out, so
  • wasm2c needs to force the C compiler not to optimize out a load, which is done with an empty inline asm statement that marks the result of the load as "read,"
  • but (unlike with a 32-bit or 64-bit register), there's no architecture-independent way to describe a 128-bit SIMD register, so at present we only have this for SIMD on x86.

Would happily take a contribution of the appropriate asm for aarch64 (possibly "w" per https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html) if somebody is able to test and say it works... I do wish we had CI tests on aarch64 given the interest in this architecture.

@sbc100
Copy link
Member

sbc100 commented May 25, 2023

Should we update the TODO such that it will #warning or #error when building on unsupported arch?

Also, I guess we should skip these tests on those platforms too..

@sbc100
Copy link
Member

sbc100 commented May 25, 2023

@rathann do you actually care about using the SIMD support in wasm2c on arm64 or are you just interested in getting the tests to pass?

@rathann
Copy link
Contributor Author

rathann commented May 26, 2023

Personally, it's the latter. We do have a growing interest in WebAssembly in Fedora, so others might actually be interested. I'll ask the other SIG members.

@rathann rathann changed the title wasm2c/spec/simd_address.txt fails on aarch64 wasm2c/spec/simd_address.txt fails on non-x86 May 26, 2023
@rathann
Copy link
Contributor Author

rathann commented May 26, 2023

I see this on ppc64le and s390x, too.

@SoniEx2 SoniEx2 linked a pull request Feb 18, 2025 that will close this issue
Open
@rathann
Copy link
Contributor Author

rathann commented Feb 21, 2025

Looks like this is fixed on s390x, now that I'm running the tests using cmake --target run-tests.

@SoniEx2
Copy link
Collaborator

SoniEx2 commented Feb 21, 2025

indeed, s390x doesn't use guard pages and falls back to range checking, while this issue only occurs with guard pages. #2545 would fix it everywhere.

@rathann rathann mentioned this issue Mar 4, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Switch to portable SIMD reads SoniEx2/wasm2kotlin
4 participants
@keithw @sbc100 @SoniEx2 @rathann