cranelift: Add a new, optional flag for preserving frame pointers

[fitzgen]

This flag is optional, but when it is set then Cranelift backends must preserve frame pointers (or ISA equivalent).

Preserving frame pointers -- even inside leaf functions -- makes it easy to capture the stack of a running program, without requiring any side tables or metadata (like .eh_frame sections). Many sampling profilers and similar tools walk frame pointers to capture stacks. Enabling this option will play nice with those tools.

Our x64 backend already unconditionally preserves frame pointers, so it is trivially conforming with this new flag.

Our aarch64 backend preserves frame pointers or not based on whether the machinst framework tells it to or not by calling gen_prologue_frame_setup, and the machinst framework looks at this new flag when deciding whether to call the backend's implementation of gen_prologue_frame_setup, so it is conforming with this new flag.

However, our s390x backend has an empty implementation of gen_prologue_frame_setup, and so it is not preserving frame pointers when this new flag is set. We will need a follow up PR that brings our s390x backend into conformance with this new flag.

cc @uweigand, your help with this last bit would be very appreciated, as I'm not familiar with s390x.

cc #4431

[cfallin]

LGTM, and good to merge once we have s390x supported as well -- thanks!

[fitzgen]

@cfallin

good to merge once we have s390x supported as well

As I just said over in #4431 (comment) I think we can actually land this PR now, since it doesn't break anything by itself, as nothing is relying on the frame pointers yet.

But we shouldn't land #4431 until there is s390x support for this flag, for sure, as that would break Wasmtime on s390x.

[cfallin]

Ah, my mistake, I had misunderstood the intended order of changes; yep, r+ for merging this now. Thanks!

[github-actions]

Subscribe to Label Action

cc @peterhuene

Details

This issue or pull request has been labeled: "cranelift", "cranelift:area:machinst", "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.

[uweigand]

cc @uweigand, your help with this last bit would be very appreciated, as I'm not familiar with s390x.

Sure, I will post a follow-up PR to add s390x support as soon as this is merged.

[fitzgen]

Sure, I will post a follow-up PR to add s390x support as soon as this is merged.

Thanks!

[bjorn3]

LLVM has a tri-state value of "always", "non-leaf" and "may-omit". It would be nice if Cranelift could have the same so I don't have to use the equivalent of "always" if "non-leaf" was chosen by the rustc target. For example Apple AArch64 targets require "non-leaf".

[fitzgen]

This is something we can do in the future as needed.

[akirilov-arm]

Actually with the way the AArch64 backend works right now it is either "always" (i.e. preserve_frame_pointers being true) or "non-leaf" (i.e. false value). "may-omit" sounds as if it would be fine to treat it as "non-leaf".

P.S. Technically even leaf functions always preserve frame pointers as long as they use the stack, e.g. define an explicit stack slot.

[bjorn3]

The name preserve_frame_pointers makes it seem like false means that frame pointers may always be omitted.

[akirilov-arm]

Yes, thinking a bit more about this, actually the AArch64 backend always "preserves" the frame pointer (i.e. it always has a valid value - either 0 or an address that points to a frame record on the stack), since the respective register is not allocatable; it is just that some functions (simple leaf functions that do not use the stack) opt not to update it, and keep its value as set by their callers.