Stream: git-wasmtime

Topic: wasmtime / PR #1522 Implement stack_addr for AArch64

view this post on Zulip Wasmtime GitHub notifications bot (Apr 16 2020 at 17:28):

bjorn3 opened PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 16 2020 at 18:58):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 16 2020 at 19:17):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 16 2020 at 19:22):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 16 2020 at 19:22):

bjorn3 created PR Review Comment:

This doesn't match the actually emitted code. The actual code uses sub for large negative offsets, while this shows add. This is because I used mem_finalize_for_show in the printing code, but manually emitted the right instructions in emit.rs. Also the mem_finalize code should probably optimize this to a sub of the absolute value of the offset when the offset is negative.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 09:50):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 09:51):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 09:51):

bjorn3 created PR Review Comment:

Fixed

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 10:06):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 11:50):

bnjbvr requested cfallin for a review on PR #1522.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 16:45):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 16:45):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 16:45):

cfallin created PR Review Comment:

Eventually I'd like to figure out a way to factor out the shared logic between emit() and show_rru() for the pseudo-instructions that lower to real instructions, without going back to the expensive edit-in-place-to-legalize paradigm; but for now this is OK, I think.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 16:45):

cfallin created PR Review Comment:

This is fine, but maybe expand the comment a bit to explain (AArch64 backend uses inline constants rather than a separate constant pool for now; we don't want to show these in the disassembly).

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 17:44):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 17:44):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 17:44):

bjorn3 created PR Review Comment:

we don't want to show these in the disassembly

This actually makes them show up as .byte directives. Before the function was truncated to just before the inline constant. I expanded the comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 17:45):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 17:45):

bjorn3 created PR Review Comment:

Yeah, it would be nice to share this.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 18:20):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 18:20):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 18:20):

cfallin created PR Review Comment:

spelling nit: s/inlie/inline/

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 18:45):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 18:46):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 17 2020 at 18:46):

bjorn3 created PR Review Comment:

Fixed

view this post on Zulip Wasmtime GitHub notifications bot (Apr 18 2020 at 11:24):

bjorn3 updated PR #1522 from aarch64-stack_addr to master:

This includes both https://github.com/cfallin/wasmtime/pull/11 and https://github.com/cfallin/wasmtime/pull/12.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 19 2020 at 02:53):

cfallin merged PR #1522.

Last updated: Dec 23 2024 at 12:05 UTC