Stream: git-wasmtime

Topic: wasmtime / PR #2960 Cranelift AArch64: Simplify leaf func...

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 11:42):

akirilov-arm opened PR #2960 from leaf_functions to main:

Leaf functions that do not use the stack (e.g. do not clobber any callee-saved registers) do not need a frame record; this has been discussed in issue #1148. I am not familiar with the ABIs of other architectures, so I don't know if it is safe to apply the same optimization, and that's why only the AArch64 backend does it.

@cfallin I'd appreciate any feedback on how these changes interact with unwinding; in particular, do we need an Inst::Unwind pseudo-instruction for the simple leaf functions we are optimizing?

cc @abrown @bnjbvr @uweigand

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 12:56):

akirilov-arm updated PR #2960 from leaf_functions to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 03 2021 at 02:50):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 03 2021 at 02:50):

cfallin created PR review comment:

Strictly speaking, this is always false on s390x, I think (though its generated prologues/epilogues are empty so it does not actually matter what we return here)?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 03 2021 at 02:50):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 03 2021 at 02:50):

cfallin created PR review comment:

If I am understanding the refactoring in the common abi_impl.rs code correctly, it seems that this should basically Just Work on x64 as well, I think?

Though it's strictly out of scope for a "simplify leaf functions on aarch64" PR, since the code is shared I think I'd prefer to retain the correspondence here and turn this leaf-function mode on for both platforms. Would you mind trying this out, if it's not too much trouble?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 10 2021 at 18:46):

akirilov-arm submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 10 2021 at 18:46):

akirilov-arm created PR review comment:

Sure, I can change the return value to false, but I can't test. @uweigand, any objections?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2021 at 11:15):

akirilov-arm submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2021 at 11:15):

akirilov-arm created PR review comment:

I replaced the check here with the one from the AArch64 backend (and made another similar change), but unfortunately it was not enough and I started getting test failures. However, that made me realize that there were no unwind test for AArch64 (such as these), neither were the GC tests enabled, so now I am not sure that even my AArch64 implementation is complete. I'll make some testing changes in separate pull requests and then I'll come back to this.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2021 at 14:12):

akirilov-arm updated PR #2960 from leaf_functions to main.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 23 2021 at 19:38):

akirilov-arm updated PR #2960 from leaf_functions to main.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2021 at 10:37):

akirilov-arm updated PR #2960 from leaf_functions to main.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 24 2021 at 11:01):

akirilov-arm updated PR #2960 from leaf_functions to main.

Last updated: Oct 23 2024 at 20:03 UTC