Stream: git-wasmtime

Topic: wasmtime / PR #2960 Cranelift: Simplify leaf functions th...

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

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

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

akirilov-arm submitted PR review.

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

akirilov-arm created PR review comment:

I also added the x64 counterpart to my changes in a separate commit, which passed the tests locally.

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

abrown submitted PR review.

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

abrown created PR review comment:

I can't say I have looked into this much but isn't there some way for the unwind info to still unwind leaf functions? Looking at the test where MacOS fails, I think this problem will also exist for aarch64?

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

akirilov-arm submitted PR review.

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

akirilov-arm created PR review comment:

Well, doesn't the comment next to the code imply that unwinding on Apple silicon is broken irrespective of my changes, so any potential breakages introduced by my patch on that platform shouldn't be a blocker to progressing with the PR (and we don't test that configuration in CI anyway)?

On the other hand, AFAIK there are no known issues with unwinding on x86-64 macOS.

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

bnjbvr submitted PR review.

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

bnjbvr created PR review comment:

unwinding on Apple silicon is broken

Right now, unwinding on M1 doesn't panic/crash, it is just materializing an incomplete backtrace, and I would be interested in at least keeping it not crashing. So I'd be happy to try your patch on Apple Silicon, fwiw! (Also it makes sense to me to get this optimization in for aarch64 only at start, and then trying to untangle what's going on on x64.)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 26 2021 at 16:18):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 26 2021 at 16:43):

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

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

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

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

bnjbvr merged PR #2960.

Last updated: Nov 22 2024 at 17:03 UTC