Implement splat instruction for x86 by abrown · Pull Request #833 · bytecodealliance/cranelift

abrown · 2019-07-08T23:21:06Z

This is a work in progress and not ready to merge but I am opening the PR for discussion (see #803). I will highlight my top questions in the code below.

[edit] This should be ready for a real review now; I've resolved all of the discussion issues.

cranelift-codegen/meta/src/isa/x86/encodings.rs

filetests/isa/x86/pinsr.clif

cranelift-codegen/meta/src/isa/x86/legalize.rs

cranelift-codegen/meta/src/cdsl/type_inference.rs

cranelift-codegen/meta/src/gen_legalizer.rs

abrown · 2019-07-09T22:37:08Z

@sunfishcode, one other thought: is there a way to guard this feature under some flag until we can properly test that all of the encodings are correct by compiling and running some WASM code?

cranelift-codegen/meta/src/isa/x86/legalize.rs

cranelift-codegen/meta/src/isa/x86/recipes.rs

cranelift-codegen/meta/src/isa/x86/legalize.rs

sunfishcode · 2019-07-09T23:29:56Z

@sunfishcode, one other thought: is there a way to guard this feature under some flag until we can properly test that all of the encodings are correct by compiling and running some WASM code?

We could use the enable_simd setting for this. It's defaulted to true right now, but we could set it to false while the SIMD code is under development to prevent frontends from accidentally using it.

filetests/isa/x86/legalize-splat.clif

abrown · 2019-07-10T22:08:23Z

@bnjbvr, @sunfishcode, @bjorn3: this should be ready for a real review now. Do I need to rebase and squash or will you squash when you merge from here?

bnjbvr · 2019-07-11T09:27:14Z

Nice. I would personally like it if there was a way to squash some of your commits together, so:

each represent a unit of work
that properly compiles and passes tests

This might be hard to do after the fact, but in some cases it's possible to do so, which would help make the review much simpler. Does it make sense to you to try to do so?

@bnjbvr

Contains fixes from @bnjbvr to codegen as a part of bnjbvr#2; necessary for SIMD features to work

cranelift-codegen/meta/src/isa/x86/settings.rs

cranelift-codegen/meta/src/shared/instructions.rs

Also, ties SIMD ISA predicates to the shared enable_simd setting

cranelift-codegen/src/isa/x86/abi.rs

abrown · 2019-07-11T20:37:05Z

@bjorn3's comments are addressed (do you have more?) and I rebased the commits to something that should make more sense in a git history, running test-all.sh after each. I do have one pending question about argument assignment (above) if anyone has an opinion.

cranelift-codegen/meta/src/isa/x86/encodings.rs

cranelift-codegen/meta/src/isa/x86/legalize.rs

cranelift-codegen/meta/src/shared/instructions.rs

cranelift-codegen/meta/src/cdsl/types.rs

cranelift-codegen/meta/src/isa/x86/encodings.rs

cranelift-codegen/meta/src/isa/x86/legalize.rs

cranelift-codegen/meta/src/isa/x86/recipes.rs

cranelift-codegen/meta/src/shared/instructions.rs

cranelift-codegen/src/ir/immediates.rs

bjorn3

LGTM

Moves scalar values in a GPR register to an FPR register

Casts bits as a different type of the same width with no change to the data (unlike bitcast)

This includes both PSHUFD and PSHUFB; these are necessary to legalize future SIMD instructions.

cranelift-codegen/src/isa/x86/abi.rs

sunfishcode

Looks good! And thanks so much @bjorn3 for the review here!

abrown commented Jul 8, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/encodings.rs Outdated Show resolved Hide resolved

abrown commented Jul 8, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/encodings.rs Outdated Show resolved Hide resolved

abrown commented Jul 8, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/encodings.rs Outdated Show resolved Hide resolved

abrown commented Jul 8, 2019

View reviewed changes

filetests/isa/x86/pinsr.clif Outdated Show resolved Hide resolved

abrown commented Jul 8, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/legalize.rs Outdated Show resolved Hide resolved

bjorn3 reviewed Jul 9, 2019

View reviewed changes

cranelift-codegen/meta/src/cdsl/type_inference.rs Outdated Show resolved Hide resolved

cranelift-codegen/meta/src/gen_legalizer.rs Show resolved Hide resolved

cranelift-codegen/meta/src/gen_legalizer.rs Show resolved Hide resolved

abrown force-pushed the add-splat-x86 branch from 18b4552 to 9f9e757 Compare July 9, 2019 20:51

abrown commented Jul 9, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/legalize.rs Outdated Show resolved Hide resolved

abrown commented Jul 9, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/recipes.rs Show resolved Hide resolved

abrown commented Jul 9, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/legalize.rs Outdated Show resolved Hide resolved

abrown commented Jul 10, 2019

View reviewed changes

filetests/isa/x86/legalize-splat.clif Show resolved Hide resolved

abrown changed the title ~~WIP: Implement splat instruction for x86~~ Implement splat instruction for x86 Jul 10, 2019

Prepare legalizer codegen for SIMD features

673c6e5

Contains fixes from @bnjbvr to codegen as a part of bnjbvr#2; necessary for SIMD features to work

abrown force-pushed the add-splat-x86 branch from f70255e to 673c6e5 Compare July 11, 2019 17:10

Disable SIMD features by default

8f4f0a0

bjorn3 reviewed Jul 11, 2019

View reviewed changes

cranelift-codegen/meta/src/isa/x86/settings.rs Outdated Show resolved Hide resolved

bjorn3 suggested changes Jul 11, 2019

View reviewed changes

Add x86-specific SIMD settings, e.g. SSE2

076b398

Also, ties SIMD ISA predicates to the shared enable_simd setting

abrown commented Jul 11, 2019

View reviewed changes

cranelift-codegen/src/isa/x86/abi.rs Outdated Show resolved Hide resolved

abrown force-pushed the add-splat-x86 branch from f71bc30 to 24e108a Compare July 11, 2019 20:34

bjorn3 reviewed Jul 12, 2019

View reviewed changes

abrown force-pushed the add-splat-x86 branch 3 times, most recently from a36cffa to 03c7803 Compare July 12, 2019 17:16

bjorn3 approved these changes Jul 12, 2019

View reviewed changes

abrown added 7 commits July 16, 2019 12:27

Assign vector arguments to FPR registers

6260290

Allow CDSL instructions to bind to vector types

9f67d91

Add scalar_to_vector instruction

a004fe0

Moves scalar values in a GPR register to an FPR register

Add raw_bitcast instruction

bf55818

Casts bits as a different type of the same width with no change to the data (unlike bitcast)

Add x86-specific shuffle instructions

d009146

This includes both PSHUFD and PSHUFB; these are necessary to legalize future SIMD instructions.

Add x86 implementation of insertlane instruction

5f0aeaf

Add x86 implementation of splat instruction

ec1c837

abrown force-pushed the add-splat-x86 branch from 03c7803 to ec1c837 Compare July 16, 2019 19:29

abrown commented Jul 16, 2019

View reviewed changes

cranelift-codegen/src/isa/x86/abi.rs Show resolved Hide resolved

sunfishcode approved these changes Jul 17, 2019

View reviewed changes

sunfishcode merged commit a5b17e8 into bytecodealliance:master Jul 17, 2019

abrown deleted the add-splat-x86 branch July 17, 2019 00:08

abrown mentioned this pull request Jul 17, 2019

Implement extractlane instruction for x86 #855

Merged

bjorn3 mentioned this pull request Jul 27, 2019

std::arch SIMD intrinsics rust-lang/rustc_codegen_cranelift#171

Open

Conversation

abrown commented Jul 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abrown commented Jul 9, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sunfishcode commented Jul 9, 2019

Uh oh!

Uh oh!

abrown commented Jul 10, 2019

Uh oh!

bnjbvr commented Jul 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abrown commented Jul 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bjorn3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sunfishcode left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

abrown commented Jul 8, 2019 •

edited

Loading