Wasmtime GitHub notifications bot (Apr 26 2022 at 01:32): Wasmtime GitHub notifications bot (Apr 26 2022 at 01:32):cfallin opened PR #4071 from x64-load-op-store to main:
These lowerings use the
OP [mem], regforms (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.
Wasmtime GitHub notifications bot (Apr 26 2022 at 01:32):cfallin requested fitzgen for a review on PR #4071.
Wasmtime GitHub notifications bot (Apr 26 2022 at 01:32):cfallin requested abrown for a review on PR #4071.
Wasmtime GitHub notifications bot (Apr 26 2022 at 01:34):cfallin edited PR #4071 from x64-load-op-store to main:
These lowerings use the
OP [mem], regforms (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.
Wasmtime GitHub notifications bot (Apr 26 2022 at 16:51):cfallin updated PR #4071 from x64-load-op-store to main.
Wasmtime GitHub notifications bot (Apr 26 2022 at 17:25):abrown submitted PR review.
Wasmtime GitHub notifications bot (Apr 26 2022 at 17:25):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.
Wasmtime GitHub notifications bot (Apr 26 2022 at 20:27):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 26 2022 at 20:27):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 26 2022 at 20:27):fitzgen created PR review comment:
Newlines between these for readability and consistency with others
Wasmtime GitHub notifications bot (Apr 26 2022 at 20:27):fitzgen created PR review comment:
Strange that the first occurrence of
flagshas 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.
Wasmtime GitHub notifications bot (Apr 26 2022 at 20:43):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Apr 26 2022 at 20:43):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.
Wasmtime GitHub notifications bot (Apr 26 2022 at 21:22):cfallin updated PR #4071 from x64-load-op-store to main.
Wasmtime GitHub notifications bot (Apr 26 2022 at 21:23):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Apr 26 2022 at 21:23):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.
Wasmtime GitHub notifications bot (Apr 26 2022 at 21:23):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Apr 26 2022 at 21:23):cfallin created PR review comment:
Fixed.
Wasmtime GitHub notifications bot (Apr 26 2022 at 23:41):cfallin updated PR #4071 from x64-load-op-store to main.
Wasmtime GitHub notifications bot (Apr 27 2022 at 00:13):cfallin updated PR #4071 from x64-load-op-store to main.
Wasmtime GitHub notifications bot (Apr 27 2022 at 01:58):cfallin merged PR #4071.
Last updated: Nov 22 2024 at 17:03 UTC