cfallin requested abrown for a review on PR #4069.
cfallin requested fitzgen for a review on PR #4069.
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.
fitzgen submitted PR review.
cfallin merged PR #4069.
abrown submitted PR review.
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 instructionsx64_movrm
andx64_xmm_movrm
would correspond to (especially in thex64_xmm_movrm
case which takes an opcode!) so they probably shouldn't have thex64_
prefix.
cfallin submitted PR review.
cfallin created PR review comment:
Urgh, I'm not sure what my fingers were thinking here -- that should be
alurm
, notmovrm
(I was probably looking at the other enum arm with a memory destination).AluRM
by analogy with the currentAluRmiR
(the latter is "reg, mem, imm to reg"; the former is "reg to mem").
cfallin submitted PR review.
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
andx64_xmm_store
maybe)?
cfallin edited PR review comment.
Last updated: Dec 23 2024 at 12:05 UTC