Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Improve extractlane and replacelane on x86#943

Merged
abrown merged 8 commits into
bytecodealliance:masterfrom
abrown:improve-extractlane
Sep 10, 2019
Merged

Improve extractlane and replacelane on x86#943
abrown merged 8 commits into
bytecodealliance:masterfrom
abrown:improve-extractlane

Conversation

@abrown

@abrown abrown commented Aug 26, 2019

Copy link
Copy Markdown
Member

Since cranelift stores float values in XMM registers there is no need for any register movement when inserting/extracting a float into/from a vector, which are also stored in XMM registers. This change depends on #868 and adds several x86-specific instructions in order to legalize the cranelift instructions to x86.

@abrown abrown force-pushed the improve-extractlane branch 3 times, most recently from 8712c78 to 2c902c3 Compare August 27, 2019 20:32
@bnjbvr bnjbvr self-requested a review August 28, 2019 16:35
@abrown abrown force-pushed the improve-extractlane branch from 2c902c3 to 42d0ab4 Compare September 4, 2019 23:15
@abrown

abrown commented Sep 4, 2019

Copy link
Copy Markdown
Member Author

@bnjbvr rebased to fix conflicts.

@bnjbvr bnjbvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great job. I think there might be some simplifications, maybe some bugs.

Comment thread cranelift-codegen/src/lib.rs Outdated
Comment thread cranelift-codegen/meta/src/isa/x86/encodings.rs Outdated
Comment thread cranelift-codegen/meta/src/isa/x86/encodings.rs Outdated
Comment thread cranelift-codegen/meta/src/isa/x86/encodings.rs Outdated
Comment thread cranelift-wasm/src/code_translator.rs
Comment thread cranelift-codegen/meta/src/isa/x86/instructions.rs
Comment thread cranelift-codegen/src/isa/x86/enc_tables.rs Outdated
Comment thread cranelift-codegen/src/isa/x86/enc_tables.rs
Comment thread cranelift-codegen/meta/src/isa/x86/encodings.rs
Comment thread filetests/isa/x86/insertlane-run.clif
@abrown abrown force-pushed the improve-extractlane branch 4 times, most recently from 10fc6eb to b2e9e09 Compare September 5, 2019 23:13
@abrown abrown requested a review from bnjbvr September 6, 2019 16:21
@abrown abrown force-pushed the improve-extractlane branch from b2e9e09 to 0f04418 Compare September 9, 2019 16:44

@bnjbvr bnjbvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks! My only strong ask before merging would be to make sure that each commit compiles properly and all tests pass on each commit, just to not break bisection in the future. If that's too much hassle, feel free to use the squash hammer.

Comment thread cranelift-codegen/meta/src/isa/x86/encodings.rs
Comment thread cranelift-codegen/meta/src/isa/x86/instructions.rs
Comment thread cranelift-codegen/meta/src/isa/x86/encodings.rs
Comment thread cranelift-codegen/meta/src/isa/x86/instructions.rs
Comment thread cranelift-codegen/src/isa/x86/enc_tables.rs
Comment thread filetests/isa/x86/insertlane-run.clif
Comment thread cranelift-codegen/meta/src/shared/instructions.rs Outdated
raw_bitcast matches the intent of this legalization more clearly (to simply change the CLIF type without changing any bits) and the additional null encodings added are necessary for later instructions
…float vector

This commit is based on the assumption that floats are already stored in XMM registers in x86. When extracting a lane, cranelift was moving the float to a regular register and back to an XMM register; this change avoids this by shuffling the float value to the lowest bits of the XMM register. It also assumes that the upper bits can be left as is (instead of zeroing them out).
@abrown abrown force-pushed the improve-extractlane branch from 0f04418 to 8255ef0 Compare September 10, 2019 17:25
@abrown abrown merged commit a88da28 into bytecodealliance:master Sep 10, 2019
@abrown abrown deleted the improve-extractlane branch September 10, 2019 17:45
walac added a commit to walac/cranelift that referenced this pull request Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants