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

Support IBM z/Architecture #2874

Merged
merged 1 commit into from May 10, 2021
Merged

Conversation

uweigand
Copy link
Member

@uweigand uweigand commented May 3, 2021

This adds support for the IBM z/Architecture (s390x-ibm-linux).

The status of the s390x backend in its current form is:

  • Wasmtime is fully functional and passes all tests on s390x.
  • All back-end features supported, with the exception of SIMD.
  • There is still a lot of potential for performance improvements.
  • Currently the only supported processor type is z15.

@cfallin cfallin self-assigned this May 3, 2021
@cfallin
Copy link
Member

cfallin commented May 3, 2021

Thanks for this @uweigand -- I'm very much looking forward to seeing this landed! I'll start reviewing soon; hopefully it won't take more than a week or two.

As a general note for others -- we have talked about the CI situation here, and for now there is still a bit of a gap in qemu's z15 support, so we won't yet have this tested on CI and will have to trust the filetests instead. This will hopefully be improved at some point, but it shouldn't stop us from considering this merge now, IMHO.

@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:meta Everything related to the meta-language. labels May 3, 2021
@uweigand uweigand force-pushed the s390x-backend branch 3 times, most recently from 079719b to f82fa6d Compare May 3, 2021 19:17
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic work -- thank you @uweigand! I'm happy to finally see this patch after lots of anticipation over the past year :-)

I have a few comments below from a skim with more detailed reading in a few key spots -- the interesting issues are more likely to be in the glue and helpers than in the bulk of the lowering patterns, and the latter should be reasonably tested by the spec testsuite, which you have reported passes with this backend.

Others please weigh in here as well -- it's a lot of code to look over! -- but on first pass this should be an easy LGTM pending a few small comments below. Thanks again!

cranelift/codegen/src/isa/s390x/abi.rs Show resolved Hide resolved
cranelift/codegen/src/isa/s390x/inst/mod.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/s390x/inst/mod.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/s390x/inst/mod.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/s390x/lower.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/s390x/lower.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/s390x/lower.rs Outdated Show resolved Hide resolved
This adds support for the IBM z/Architecture (s390x-ibm-linux).

The status of the s390x backend in its current form is:
- Wasmtime is fully functional and passes all tests on s390x.
- All back-end features supported, with the exception of SIMD.
- There is still a lot of potential for performance improvements.
- Currently the only supported processor type is z15.
@uweigand
Copy link
Member Author

Thanks for the review, @cfallin! I've checked in an update that should address all points you've raised.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @uweigand -- everything appears to be resolved and I don't see any other major issues, so I think we can go ahead and merge! Of course we should improve the CI situation once we can; it would be nice to run the tests on every commit, once qemu is capable of doing that; but there's nothing particularly urgent about it. For now, ensuring that the backend builds and the filetests run should be enough to prevent major breakage.

@cfallin cfallin merged commit 5fb2c8c into bytecodealliance:main May 10, 2021
@uweigand uweigand deleted the s390x-backend branch May 11, 2021 07:52
@uweigand
Copy link
Member Author

Thanks! I'll have a look at the current state of qemu. I'll also see if I can add support for z14, which would be useful anyway and might make it easier to use qemu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:meta Everything related to the meta-language. cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants