cranelift: Add inline stack probing for x64

[afonso360]

👋 Hey,

This PR adds inline stack probing for the x64 backend. Opening as a draft since there are still some pending questions.

This is feature gated behind a enable_inline_probestack and also requires the user to enable enable_probestack.

Currently we have two implementations, If we only need to probe a few stack pages, we unroll the probes. If we need more we fall back to using a loop.

Closes: #2299
cc: @bjorn3 @cfallin

[github-actions]

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 .github/subscribe-to-label.json configuration file.

Learn more.

[jameysharp]

This needs what I hope will be an easy rebase; cranelift/codegen/src/machinst/abi_impl.rs got merged into cranelift/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]

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 and probestack_strategy. Should we merge those flags by adding a disabled arm to probestack_strategy?

[cfallin]

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.

[jameysharp]

I have no opinion on whether to merge enable_probestack and probestack_strategy; I could imagine arguments both ways.

I have a couple questions/suggestions from doing another review pass but I think this would be fine to merge regardless.

[jameysharp]

Why do just a few runtests need stack-probing disabled?

[afonso360]

These tests have stack slots above the threshold where we emit a stack probe (4k by default I think). The default strategy is to outline the stack probing to a external function call, but since we never provide an implementation for that anywhere the JIT can't link these tests, so they fail.

I manually disabled stack probing for these which reflects the old behaviour of force disabling it everywhere.