wasmtime / issue #5875 cranelift: Add atomic_cas to inter... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #5875 cranelift: Add atomic_cas to inter...

Wasmtime GitHub notifications bot (Feb 24 2023 at 15:02):

Happy to help :smiley:

Sure thing. I shall add it to the relevant test files.

As for further help on the interpreter, I am still learning to walk with Rust and grokking the project as a whole. I am, however, very keen to contribute so point me in the right direction and I will help out anyway I can :+1:.

Wasmtime GitHub notifications bot (Feb 24 2023 at 15:41):

afonso360 commented on issue #5875:

Awesome! :rocket:

I'm not 100% sure exactly what we are missing as far as SIMD goes since I haven't worked on that part in a while.

We have some special instructions that load SIMD values from memory and build a vector such as sload32x2. I know those are missing an implementation in the interpreter.

I just noticed we don't implement is_null/is_invalid/null, those are fairly easy scalar instructions, with pre existing tests.

A good exercise would be to go through the simd-* runtests and see which ones are missing the test interpret annotation, I'm betting a lot of those are already working but we forgot to enable the tests!

And if they are broken we can fix them!

And finally a while ago I did a roundup of what was missing in the interpreter, It a bit out of date, but it might help!

Wasmtime GitHub notifications bot (Feb 24 2023 at 15:41):

afonso360 edited a comment on issue #5875:

Awesome! :rocket:

I'm not 100% sure exactly what we are missing as far as SIMD goes since I haven't worked on that part in a while.

We have some special instructions that load SIMD values from memory and build a vector such as sload32x2. I know those are missing an implementation in the interpreter.

I just noticed we don't implement is_null/is_invalid/null, those are fairly easy scalar instructions, with pre existing tests.

A good exercise would be to go through the simd-* runtests and see which ones are missing the test interpret annotation, I'm betting a lot of those are already working but we forgot to enable the tests!

And if they are broken we can fix them!

And finally a while ago I did a roundup of what was missing in the interpreter, It's a bit out of date, but it might help!

Wasmtime GitHub notifications bot (Feb 25 2023 at 11:51):

afonso360 commented on issue #5875:

It looks like CI failed on s390x that's our only big-endian architecture, that's probably why its difficult to reproduce this issue locally.

I looked into this and it looks like that the way those tests are written, they depend on the interpreter being able to recognize the little & big flags on stores and loads to pass.

That is an issue because the interpreter just understands native endianness, so little is okay on x86 but big isn't. And vice versa on s390x big is okay, but little isn't.

I think for now It's okay to revert that change and make those tests not run on the interpreter. I didn't expect this would be a big issue for the little endian tests, but it makes sense that they would fail too!

Wasmtime GitHub notifications bot (Feb 25 2023 at 11:51):

afonso360 edited a comment on issue #5875:

It looks like CI failed on s390x that's our only big-endian architecture, that's probably why its difficult to reproduce this issue locally.

I looked into this and it looks like that the way those tests are written, they depend on the interpreter being able to recognize the little & big flags on stores and loads to pass.

That is an issue because the interpreter just understands native endianness, so little is okay on x86 but big isn't. And vice versa on s390x big is okay, but little isn't.

I think for now It's okay to revert that change and make the subword tests not run on the interpreter. I didn't expect this would be a big issue for the little endian tests, but it makes sense that they would fail too!

Wasmtime GitHub notifications bot (Feb 25 2023 at 12:01):

afonso360 edited a comment on issue #5875:

It looks like CI failed on s390x that's our only big-endian architecture, that's probably why its difficult to reproduce this issue locally.

I looked into this and it looks like that the way those tests are written, they depend on the interpreter being able to recognize the little & big flags on stores and loads to pass.

That is an issue because the interpreter just understands native endianness, so little is okay on x86 but big isn't. And vice versa on s390x big is okay, but little isn't.

I think for now It's okay to revert that change and make the subword tests not run on the interpreter. I didn't expect this would be a big issue for the little endian tests, but it makes sense that they would fail too!

Edit: Just opened https://github.com/bytecodealliance/wasmtime/issues/5881 which addresses this ^

Wasmtime GitHub notifications bot (Feb 25 2023 at 12:02):

afonso360 edited a comment on issue #5875:

It looks like CI failed on s390x that's our only big-endian architecture, that's probably why its difficult to reproduce this issue locally.

I looked into this and it looks like that the way those tests are written, they depend on the interpreter being able to recognize the little & big flags on stores and loads to pass.

That is an issue because the interpreter just understands native endianness, so little is okay on x86 but big isn't. And vice versa on s390x big is okay, but little isn't.

I think for now It's okay to revert that change and make the subword tests not run on the interpreter. I didn't expect this would be a big issue for the little endian tests, but it makes sense that they would fail too!

Edit: Just opened https://github.com/bytecodealliance/wasmtime/issues/5881 which addresses this ^

Wasmtime GitHub notifications bot (Feb 25 2023 at 13:53):

jan-justin commented on issue #5875:

I see, thanks for the explanation.

I will revert the changes and look into the issue you opened separately.

As for the remaining work on the interpreter, I shall reach out to you when the time is right.

Last updated: Dec 23 2024 at 12:05 UTC