cfallin opened PR #4071 from x64-load-op-store
to main
:
These lowerings use the
OP [mem], reg
forms (or in AT&T syntax,OP %reg, (mem)
) -- i.e., x86 instructions that load from memory, perform
an ALU operation, and store the result, all in one instruction. Using
these instruction forms, we can merge three CLIF ops together: a load,
an arithmetic operation, and a store.
cfallin requested fitzgen for a review on PR #4071.
cfallin requested abrown for a review on PR #4071.
cfallin edited PR #4071 from x64-load-op-store
to main
:
These lowerings use the
OP [mem], reg
forms (or in AT&T syntax,OP %reg, (mem)
) --
i.e., x86 instructions that load from memory, perform
an ALU operation, and store the result, all in one instruction. Using
these instruction forms, we can merge three CLIF ops together: a load,
an arithmetic operation, and a store.Stacks on top of #4069; last two commits are new.
cfallin updated PR #4071 from x64-load-op-store
to main
.
abrown submitted PR review.
abrown created PR review comment:
I don't think these should be presented in 3-operand form. It doesn't match x86_64, which is confusing.
fitzgen submitted PR review.
fitzgen submitted PR review.
fitzgen created PR review comment:
Newlines between these for readability and consistency with others
fitzgen created PR review comment:
Strange that the first occurrence of
flags
has the=
here rather than the second. Is that necessary?Aside: I feel like we shouldn't even have the
=
syntax, and if you use the same variable name in multiple places we automatically unify.
cfallin submitted PR review.
cfallin created PR review comment:
I was perplexed as well; it has to do with the inline extractor expanding with the arguments in a flipped order.
I agree that inferring
=
should be totally possible depending on whether the variable is already captured or not, and I don't know why we didn't think of that earlier! I'll follow up with a PR to allow that and see what folks think.
cfallin updated PR #4071 from x64-load-op-store
to main
.
cfallin submitted PR review.
cfallin created PR review comment:
Yeah, that's probably true; I was trying to mirror the SSA-ified view of the reg-reg forms but with a memory dest I think it's just as clear in "actual" form (since the memory dest is a source from a regalloc perspective). Updated.
cfallin submitted PR review.
cfallin created PR review comment:
Fixed.
cfallin updated PR #4071 from x64-load-op-store
to main
.
cfallin updated PR #4071 from x64-load-op-store
to main
.
cfallin merged PR #4071.
Last updated: Dec 23 2024 at 12:05 UTC