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

Cranelift: Get tail calls working on aarch64 #6723

Merged
merged 2 commits into from
Jul 22, 2023

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Jul 13, 2023

No description provided.

@fitzgen fitzgen requested a review from cfallin July 13, 2023 19:01
@fitzgen fitzgen requested a review from a team as a code owner July 13, 2023 19:01
@fitzgen fitzgen removed the request for review from a team July 13, 2023 19:01
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen isle Related to the ISLE domain-specific language labels Jul 13, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

  • cfallin: isle
  • fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@fitzgen fitzgen requested review from jameysharp and removed request for cfallin July 13, 2023 20:14
Copy link
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

I haven't reviewed all of this but ran out of time for today. Overall I think it's right but would like somebody else to take a look. For now I have one minor suggestion.

cranelift/codegen/src/isa/aarch64/inst/emit.rs Outdated Show resolved Hide resolved
@fitzgen fitzgen force-pushed the aarch64-tail-calls branch from 6d00449 to 779f768 Compare July 14, 2023 20:52
@afonso360
Copy link
Contributor

afonso360 commented Jul 16, 2023

Could you also enable tail calls for AArch64 on fuzzgen, here? I've been fuzzing this over that past couple of days and it seems fairly stable.

@fitzgen fitzgen force-pushed the aarch64-tail-calls branch from 779f768 to e96a11c Compare July 17, 2023 16:57
@fitzgen
Copy link
Member Author

fitzgen commented Jul 17, 2023

Could you also enable tail calls for AArch64 on fuzzgen, here? I've been fuzzing this over that past couple of days and it seems fairly stable.

Good catch, thanks!

Done.

@fitzgen fitzgen force-pushed the aarch64-tail-calls branch from e96a11c to 1a1a1bc Compare July 18, 2023 20:29
@fitzgen fitzgen requested review from jameysharp and elliottt and removed request for jameysharp July 19, 2023 21:20
Copy link
Member

@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

This looks great to me, thank you for all the comments!

cranelift/codegen/src/isa/aarch64/inst/emit.rs Outdated Show resolved Hide resolved
@fitzgen fitzgen enabled auto-merge July 21, 2023 18:13
@afonso360 afonso360 disabled auto-merge July 21, 2023 20:11
@afonso360
Copy link
Contributor

Oops, miss click sorry

@afonso360 afonso360 enabled auto-merge July 21, 2023 20:11
@afonso360 afonso360 added this pull request to the merge queue Jul 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 22, 2023
@fitzgen fitzgen added this pull request to the merge queue Jul 22, 2023
Merged via the queue into bytecodealliance:main with commit 8480b9b Jul 22, 2023
@fitzgen fitzgen deleted the aarch64-tail-calls branch July 22, 2023 18:37
geekbeast pushed a commit to geekbeast/wasmtime that referenced this pull request Aug 1, 2023
* main: (47 commits)
  Add core dump support to the runtime (bytecodealliance#6513)
  Resource table tracks child relationships (bytecodealliance#6779)
  Wasmtime: Move `OnDemandInstanceAllocator` to its own module (bytecodealliance#6790)
  wasi: Test the stdio streams implementation (bytecodealliance#6764)
  Don't generate same-named imports in fact modules (bytecodealliance#6783)
  Wasmtime: Add support for Wasm tail calls (bytecodealliance#6774)
  Cranelift: Fix `ABIMachineSpec::gen_add_imm` for riscv64 (bytecodealliance#6780)
  Update the wasm-tools family of crates, disallow empty component types (bytecodealliance#6777)
  Fix broken link to WASI API documentation (bytecodealliance#6775)
  A bunch of cleanups for cranelift-codegen-meta (bytecodealliance#6772)
  Implement component-to-component calls with resources (bytecodealliance#6769)
  Ignore async_stack_size if async_support is disabled (bytecodealliance#6771)
  A bunch of minor cleanups (bytecodealliance#6767)
  Fix flaky tests in preview2 streams (bytecodealliance#6763)
  Refactor and simplify component trampolines (bytecodealliance#6751)
  Cranelift: Implement tail calls on riscv64 (bytecodealliance#6749)
  WASI Preview 2: rewrite streams and pollable implementation (bytecodealliance#6556)
  cranelift-wasm: Add support for translating Wasm tail calls to CLIF (bytecodealliance#6760)
  Cranelift: Get tail calls working on aarch64 (bytecodealliance#6723)
  Implement component model resources in Wasmtime (bytecodealliance#6691)
  ...
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. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants