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

Enable reftypes tests on aarch64. #2028

Closed

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Jul 15, 2020

With #1852, we now have support for reference types in the new backend
framework generally, and on aarch64 in particular. This PR removes the
test-ignore directive for reftypes tests on this platform.

This actually enables reftypes support for all platforms, rather than explicitly
only on x86_64, but going forward all new backends should include reftypes
support (since the new-backend framework does, and it just takes a few
machine-specific opcodes beyond that to do so), so I think it's best just to
set this baseline now.

Fixes #1886

@cfallin cfallin added the cranelift:area:aarch64 Issues related to AArch64 backend. label Jul 15, 2020
@alexcrichton
Copy link
Member

There's an instance of #1886 in tests/all/main.rs, mind removing those #[cfg]? I think this can close #1886 as well afterwards?

In any case, very nice!

@fitzgen
Copy link
Member

fitzgen commented Jul 15, 2020

There's a few of them now, I think all of these cfgs can be removed:

tests/all/func.rs:126:#[cfg(target_arch = "x86_64")]
tests/all/func.rs:449:#[cfg(target_arch = "x86_64")]
tests/all/func.rs:484:#[cfg(target_arch = "x86_64")]
tests/all/main.rs:23:#[cfg(target_arch = "x86_64")]
tests/all/main.rs:25:#[cfg(target_arch = "x86_64")]
tests/all/main.rs:29:#[cfg(target_arch = "x86_64")]

@fitzgen
Copy link
Member

fitzgen commented Jul 15, 2020

Also, thanks! 🎉 🎉 🎉

@cfallin
Copy link
Member Author

cfallin commented Jul 15, 2020

Argh -- false positive: removing the directives in tests/all/main.rs, I see failures of gc::smoke_test_gc and gc::gc_during_gc_from_many_table_gets. I wonder if the tests enabled in build.rs are only testing opcodes and the gc tests exercise the stackwalking more?

I'll push the commit in any case so the failures are visible here on the CI run.

@fitzgen
Copy link
Member

fitzgen commented Jul 15, 2020

It is possible that we don't dynamically trigger any GC during the whole reference types proposal spec tests, and so they would only be testing the code generation.

@cfallin cfallin force-pushed the enable-aarch64-reftype-tests branch from 0a5f714 to ac5130c Compare July 15, 2020 22:00
tests/all/main.rs Outdated Show resolved Hide resolved
With bytecodealliance#1852, we now have support for reference types in the new backend
framework generally, and on aarch64 in particular. This PR removes the
test-ignore directive for reftypes tests on this platform.

Closes bytecodealliance#1886.
@cfallin cfallin force-pushed the enable-aarch64-reftype-tests branch from ac5130c to 2d45ce2 Compare July 15, 2020 22:02
@cfallin
Copy link
Member Author

cfallin commented Nov 13, 2020

Closing this now, as #2410 enables GC/reftypes on all backends!

@cfallin cfallin closed this Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasmtime: support reference types on aarch64
3 participants