Stream: git-wasmtime

Topic: wasmtime / PR #4069 x64 backend: migrate stores, and rema...

Wasmtime GitHub notifications bot (Apr 25 2022 at 22:56):

cfallin requested abrown for a review on PR #4069.

Wasmtime GitHub notifications bot (Apr 25 2022 at 22:56):

cfallin requested fitzgen for a review on PR #4069.

Wasmtime GitHub notifications bot (Apr 25 2022 at 22:56):

cfallin opened PR #4069 from x64-isle-stores to main:

This is a prerequisite to some codegen improvements I want to make involving stores specifically, so this PR migrates stores and finishes the migration of loads on x64.

Wasmtime GitHub notifications bot (Apr 26 2022 at 16:49):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 26 2022 at 16:50):

cfallin merged PR #4069.

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:34):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:34):

abrown created PR review comment:

@cfallin: I think that, in developing conventions for this ISLE lowering, we should attempt to use the ISA naming conventions for the actual instructions that get emitted. That way, anyone looking at the lowering can quickly discern what instructions will be emitted in a lowering without having to trace some enum variants through to emit.rs (if they even know to look there!). It's not clear what instructions x64_movrm and x64_xmm_movrm would correspond to (especially in the x64_xmm_movrm case which takes an opcode!) so they probably shouldn't have the x64_ prefix.

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:47):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:47):

cfallin created PR review comment:

Urgh, I'm not sure what my fingers were thinking here -- that should be alurm, not movrm (I was probably looking at the other enum arm with a memory destination). AluRM by analogy with the current AluRmiR (the latter is "reg, mem, imm to reg"; the former is "reg to mem").

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:48):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:48):

cfallin created PR review comment:

Oh -- nevermind the above; I had thought this was a comment on the ALU op-to-mem PR, not the store PR.

The MovRM forms were already existing but happy to take suggestions for better names? Or do you mean just the store helpers (e.g., x64_store and x64_xmm_store maybe)?

Wasmtime GitHub notifications bot (Apr 26 2022 at 17:48):

cfallin edited PR review comment.

Last updated: Oct 23 2024 at 20:03 UTC