Stream: git-cranelift

Topic: cranelift / Issue #1320 Refactor unwind; add FDE support.

view this post on Zulip GitHub (Jan 10 2020 at 22:16):

yurydelendik commented on Issue #1320:

I also think a follow-up PR to make test_unwind.rs more generic so that it supports FDE generation in addition to Windows unwind information would be ideal.

Yeah, currently I added "test_fde.rs" -- it will match how wasmtime will use the functionality, by using FrameUnwindKind parameter.

view this post on Zulip GitHub (Jan 10 2020 at 22:23):

peterhuene commented on Issue #1320:

:+1:

I think in a future PR we might want to unify the unwind and fde subtests so that it'll print out the right thing depending on the given function's ABI and calling convention.

I feel that unwind is too generic a term to mean just Windows unwind information, so it might lead to confusing in the long run. Perhaps something to revisit when we further abstract things.

view this post on Zulip GitHub (Jan 10 2020 at 22:23):

peterhuene edited a comment on Issue #1320:

:+1:

I think in a future PR we might want to unify the unwind and fde subtests so that it'll print out the right thing depending on the given function's ABI and calling convention.

I feel that unwind is too generic a term to mean just Windows unwind information, so it might lead to confusion in the long run. Perhaps something to revisit when we further abstract things.

view this post on Zulip GitHub (Jan 13 2020 at 20:17):

iximeow commented on Issue #1320:

sorry for the late comments - I was out last week and as a result wasn't checking cranelift too closely. I do have a few comments, but I expect them to be subsequent PRs, and would like to discuss a bit first.

view this post on Zulip GitHub (Jan 13 2020 at 21:12):

yurydelendik commented on Issue #1320:

I do have a few comments, but I expect them to be subsequent PRs

@iximeow thank you for review, I'll try to get back to them shortly after bytecodealliance/wasmtime#759 . As @peterhuene noted, there will be more refactoring connected with generalizing unwind information.

view this post on Zulip GitHub (Jan 15 2020 at 09:17):

bnjbvr commented on Issue #1320:

Random drive-by comment for a future PR: can you explicit what the acronym FDE means, at the top of the fde.rs file, for instance?

Last updated: Nov 22 2024 at 17:03 UTC