Skip to content

x64: Add fcvt_from_uint lowering for i64x2#7919

Merged
fitzgen merged 1 commit intobytecodealliance:mainfrom
alexcrichton:x64-float-lwoering
Feb 12, 2024
Merged

x64: Add fcvt_from_uint lowering for i64x2#7919
fitzgen merged 1 commit intobytecodealliance:mainfrom
alexcrichton:x64-float-lwoering

Conversation

@alexcrichton
Copy link
Member

This commit adds a general purpose lowering for the fcvt_from_uint instruction in addition to the previously specialized lowering for what the wasm frontend produces. This is unlikely to get triggered much from the wasm frontend except when intermediate optimizations change the shape of code. The goal of this commit is to address issues such as those identified in #7915 and historically by ensuring that there's a lowering for the instruction for all input types instead of trying to massage the input into the right form.

This instruction lowering was crafted by studying LLVM's output and I've put commentary to the best of my ability as to what's going on.

This commit adds a general purpose lowering for the `fcvt_from_uint`
instruction in addition to the previously specialized lowering for what
the wasm frontend produces. This is unlikely to get triggered much from
the wasm frontend except when intermediate optimizations change the
shape of code. The goal of this commit is to address issues such as
those identified in bytecodealliance#7915 and historically by ensuring that there's a
lowering for the instruction for all input types instead of trying to
massage the input into the right form.

This instruction lowering was crafted by studying LLVM's output and I've
put commentary to the best of my ability as to what's going on.
@alexcrichton alexcrichton requested a review from a team as a code owner February 12, 2024 18:20
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team February 12, 2024 18:20
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:x64 Issues related to x64 codegen labels Feb 12, 2024
Comment on lines +3434 to +3435
;; Apply some math and this should produce the same result as the native
;; conversion.
Copy link
Member

Choose a reason for hiding this comment

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

🪄

@fitzgen fitzgen added this pull request to the merge queue Feb 12, 2024
Merged via the queue into bytecodealliance:main with commit c573158 Feb 12, 2024
@alexcrichton alexcrichton deleted the x64-float-lwoering branch February 13, 2024 03:11
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Mar 12, 2024
This instruction was previously unimplemented in Cranelift as pointed
out in bytecodealliance#8084. While a fallback for `fcvt_from_uint` was implemented
in bytecodealliance#7919 I forgot to do the same for the signed version. This commit
adds a fallback that decomposes the input into two scalars and converts
each individually, then reassembling the result.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Mar 12, 2024
This instruction was previously unimplemented in Cranelift as pointed
out in bytecodealliance#8084. While a fallback for `fcvt_from_uint` was implemented
in bytecodealliance#7919 I forgot to do the same for the signed version. This commit
adds a fallback that decomposes the input into two scalars and converts
each individually, then reassembling the result.
github-merge-queue bot pushed a commit that referenced this pull request Mar 12, 2024
This instruction was previously unimplemented in Cranelift as pointed
out in #8084. While a fallback for `fcvt_from_uint` was implemented
in #7919 I forgot to do the same for the signed version. This commit
adds a fallback that decomposes the input into two scalars and converts
each individually, then reassembling the result.
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