github-actions[bot] commented on issue #4747:
Subscribe to Label Action
cc @peterhuene
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "cranelift:meta", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
jameysharp commented on issue #4747:
This needs what I hope will be an easy rebase;
cranelift/codegen/src/machinst/abi_impl.rs
got merged intocranelift/codegen/src/machinst/abi.rs
, so your changes just need to be applied to that file instead.Otherwise, I think this looks like it does what you say it should do. @cfallin, now that it's implemented as a pseudo-instruction, what do you think?
afonso360 commented on issue #4747:
Thanks for the reminder! I didn't realize this had a conflict.
There is an additional question that I'd like to ask for opinions. After this PR (as it is) we have two flags,
enable_probestack
andprobestack_strategy
. Should we merge those flags by adding adisabled
arm toprobestack_strategy
?
cfallin commented on issue #4747:
Otherwise, I think this looks like it does what you say it should do. @cfallin, now that it's implemented as a pseudo-instruction, what do you think?
Looks great to me! I think this is the right approach. I didn't look closely at the emission code but the sequence in the comments seems reasonable, as does the unrolled-vs-looping optimization.
Last updated: Dec 23 2024 at 13:07 UTC