Skip to content

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

Merged
cfallin merged 1 commit into
bytecodealliance:mainfrom
cfallin:idiv-x64-8bit-rdx
Jun 27, 2022
Merged

Cranelift/x64: fix register allocator metadata for 8-bit divides.#4332
cfallin merged 1 commit into
bytecodealliance:mainfrom
cfallin:idiv-x64-8bit-rdx

Conversation

@cfallin
Copy link
Copy Markdown
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 abrown and fitzgen June 27, 2022 18:17
@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
Copy Markdown
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@cfallin cfallin force-pushed the idiv-x64-8bit-rdx branch from 4e508c5 to 52c51a5 Compare June 27, 2022 18:42
`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 force-pushed the idiv-x64-8bit-rdx branch from 52c51a5 to 5050271 Compare June 27, 2022 18:42
@cfallin cfallin merged commit 5c2c285 into bytecodealliance:main Jun 27, 2022
@cfallin cfallin deleted the idiv-x64-8bit-rdx branch June 27, 2022 19:31
cfallin added a commit to cfallin/wasmtime that referenced this pull request 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 pull request 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 pull request 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 pull request 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.

2 participants