Skip to content

x64: Add {u,s}mulhi.i8 instruction support#7866

Merged
alexcrichton merged 1 commit intobytecodealliance:mainfrom
afonso360:x64-usmulhi
Feb 3, 2024
Merged

x64: Add {u,s}mulhi.i8 instruction support#7866
alexcrichton merged 1 commit intobytecodealliance:mainfrom
afonso360:x64-usmulhi

Conversation

@afonso360
Copy link
Contributor

👋 Hey,

This PR adds support for smulhi.i8 and umulhi.i8 to the x64 backend.

Closes #5468
Closes #7865

@afonso360 afonso360 added the cranelift:area:x64 Issues related to x64 codegen label Feb 3, 2024
@afonso360 afonso360 requested a review from a team as a code owner February 3, 2024 19:10
@afonso360 afonso360 requested review from fitzgen and removed request for a team February 3, 2024 19:10
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Feb 3, 2024
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks!

@alexcrichton alexcrichton added this pull request to the merge queue Feb 3, 2024
Merged via the queue into bytecodealliance:main with commit c7a7b8c Feb 3, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Feb 3, 2024
This commit is inspired after reading over some code from bytecodealliance#7865
and bytecodealliance#7866. The goal of this commit was to refactor
scalar multiplication-related instructions in the x64 backend to more
closely align with their native instructions. Changes include:

* The `MulHi` instruction is renamed to `Mul`. This represents either
  `mul` or `imul` producing a doublewide result.
* A `Mul8` instruction was added to correspond to `Mul` for the 8-bit
  variants that produce a doublewide result in the `AX` register rather
  than the other instructions which split between `RAX` and `RDX`.
* The `UMulLo` instruction was removed as now it's covered by `Mul`
* The `AluRmiROpcode::Mul` opcode was removed in favor of new `IMul` and
  `IMulImm` instructions. Register allocation and emission already had
  special cases for `Mul` which felt better as standalone instructions
  rather than putting in an existing variant.

Lowerings using `imul` are not affected in general but the `IMulImm`
instruction has different register allocation behavior than before which
allows the destination to have a different register than the first
operand. The `umulhi` and `smulhi` instructions are also reimplemented
with their 8-bit variants instead of extension-plus-16-bit variants.
github-merge-queue bot pushed a commit that referenced this pull request Feb 9, 2024
* x64: Refactor multiplication instructions

This commit is inspired after reading over some code from #7865
and #7866. The goal of this commit was to refactor
scalar multiplication-related instructions in the x64 backend to more
closely align with their native instructions. Changes include:

* The `MulHi` instruction is renamed to `Mul`. This represents either
  `mul` or `imul` producing a doublewide result.
* A `Mul8` instruction was added to correspond to `Mul` for the 8-bit
  variants that produce a doublewide result in the `AX` register rather
  than the other instructions which split between `RAX` and `RDX`.
* The `UMulLo` instruction was removed as now it's covered by `Mul`
* The `AluRmiROpcode::Mul` opcode was removed in favor of new `IMul` and
  `IMulImm` instructions. Register allocation and emission already had
  special cases for `Mul` which felt better as standalone instructions
  rather than putting in an existing variant.

Lowerings using `imul` are not affected in general but the `IMulImm`
instruction has different register allocation behavior than before which
allows the destination to have a different register than the first
operand. The `umulhi` and `smulhi` instructions are also reimplemented
with their 8-bit variants instead of extension-plus-16-bit variants.

* Remove outdated emit tests

These are all covered by the filetests framework now too.

* Fix Winch build
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

2 participants