fitzgen opened PR #4469 from preserve-frame-pointers-flag
to main
:
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 themachinst
framework tells it to or not by callinggen_prologue_frame_setup
, and themachinst
framework looks at this new flag when deciding whether to call the backend's implementation ofgen_prologue_frame_setup
, so it is conforming with this new flag.However, our
s390x
backend has an empty implementation ofgen_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 ours390x
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 https://github.com/bytecodealliance/wasmtime/pull/4431
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
fitzgen requested cfallin for a review on PR #4469.
cfallin submitted PR review.
cfallin created PR review comment:
Can we add the equivalent of this "preserves frame pointers" test for x64 too? The flag won't actually matter now but it'd be good to have the test coverage if/when we add any omit-frame-pointers mode in the future.
cfallin submitted PR review.
fitzgen updated PR #4469 from preserve-frame-pointers-flag
to main
.
fitzgen submitted PR review.
fitzgen created PR review comment:
Done!
fitzgen merged PR #4469.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
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 submitted PR review.
fitzgen created PR review comment:
This is something we can do in the future as needed.
akirilov-arm created PR review comment:
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".
akirilov-arm submitted PR review.
akirilov-arm edited PR review comment.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
The name preserve_frame_pointers makes it seem like false means that frame pointers may always be omitted.
akirilov-arm submitted PR review.
akirilov-arm created PR review comment:
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.
Last updated: Dec 23 2024 at 12:05 UTC