Stream: git-wasmtime

Topic: wasmtime / PR #10648 asm: clear GPR with `xor <tmp>, <tmp>`

Wasmtime GitHub notifications bot (Apr 22 2025 at 22:23):

abrown opened PR #10648 from abrown:asm-xor-clear to bytecodealliance:main:

This change uses an "uninitialized value" to resolve the register allocation issues that come up when clearing a GPR with xor; this tries out the approach suggested in #10632 and will replace that if merged. The problem is that, in xor <tmp>, <tmp>, we need some way to communicate to the register allocator that the previous values of <tmp> are completely cleared--i.e., not used. While #10632 started down the path of allowing the register allocator to make a different decision about <tmp>--the approach currently taken for AluConstOp--this change instead adopts the "uninitialized value" approach. As was done previously with XmmUninitializedValue, we create a new pseudo-instruction called GprUninitializedValue whose registers are defined out of thin air.

Wasmtime GitHub notifications bot (Apr 22 2025 at 22:45):

cfallin submitted PR review:

LGTM, thanks!

Wasmtime GitHub notifications bot (Apr 22 2025 at 22:45):

cfallin created PR review comment:

I know pre-existing, but can we add a bit more here about how it's used (to ease concerns of the reader that this looks wildly unsafe)? Basically something like: "This is always immediately followed by an instruction, such as xor reg, reg, that semantically reads this undefined value but arithmetically produces the same result regardless of its value."

Wasmtime GitHub notifications bot (Apr 23 2025 at 00:06):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Apr 23 2025 at 00:06):

abrown created PR review comment:

I do have some questions about the offset changes here (hence the draft status): here, for example, we've managed to shave a byte off our encodings somehow...

Wasmtime GitHub notifications bot (Apr 23 2025 at 00:06):

abrown created PR review comment:

And here, we've somehow lost a byte again, but without any other changes to the file?

Wasmtime GitHub notifications bot (Apr 23 2025 at 00:06):

abrown created PR review comment:

...whereas here we've managed to gain two of them! It's not yet clear to me why this is.

Wasmtime GitHub notifications bot (Apr 23 2025 at 00:11):

abrown updated PR #10648.

Wasmtime GitHub notifications bot (Apr 23 2025 at 16:10):

abrown has marked PR #10648 as ready for review.

Wasmtime GitHub notifications bot (Apr 23 2025 at 16:10):

abrown requested fitzgen for a review on PR #10648.

Wasmtime GitHub notifications bot (Apr 23 2025 at 16:10):

abrown requested wasmtime-compiler-reviewers for a review on PR #10648.

Wasmtime GitHub notifications bot (Apr 23 2025 at 16:10):

abrown requested wasmtime-core-reviewers for a review on PR #10648.

Wasmtime GitHub notifications bot (Apr 23 2025 at 16:14):

abrown commented on PR #10648:

In the Cranelift meeting today we chalked these offset changes up to different register allocation (potentially adding or removing REX bytes in a few places) — no one sounded too concerned. We did discuss another approach: instead of continuing to rely on the uninit footgun, we could create a pseudo-instruction, much like the current AluConstOp, to differentiate this xor <tmp>, <tmp> pattern for register allocation. (I have built a branch that already walks out that approach: https://github.com/bytecodealliance/wasmtime/compare/main...abrown:wasmtime:asm-const).

Wasmtime GitHub notifications bot (Apr 23 2025 at 17:18):

abrown merged PR #10648.

Last updated: Dec 06 2025 at 07:03 UTC