Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cranelift/x64: fix register allocator metadata for 8-bit divides. #4332

Merged
merged 1 commit into from Jun 27, 2022

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Jun 27, 2022

idiv on x86-64 only reads rdx/edx/dx/dl for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from ax, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined rdx) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.

@cfallin cfallin requested review from fitzgen and abrown Jun 27, 2022
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:x64 Issues related to x64 codegen labels Jun 27, 2022
Copy link
Member

@fitzgen fitzgen left a comment

Good catch!

`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.
@cfallin cfallin merged commit 5c2c285 into bytecodealliance:main Jun 27, 2022
23 checks passed
@cfallin cfallin deleted the idiv-x64-8bit-rdx branch Jun 27, 2022
cfallin added a commit to cfallin/wasmtime that referenced this issue Jun 27, 2022
…tecodealliance#4332)

`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.
cfallin added a commit to cfallin/wasmtime that referenced this issue Jun 27, 2022
…tecodealliance#4332)

`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.
alexcrichton pushed a commit that referenced this issue Jun 27, 2022
* Cranelift/x64: fix register allocator metadata for 8-bit divides. (#4332)

`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.

* Update RELEASES.md.
afonso360 pushed a commit to afonso360/wasmtime that referenced this issue Jun 30, 2022
…tecodealliance#4332)

`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants