Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SIMD: implements v128 load, store and lane manipulations. #588

Merged
merged 11 commits into from
Jun 1, 2022
Merged

SIMD: implements v128 load, store and lane manipulations. #588

merged 11 commits into from
Jun 1, 2022

Conversation

mathetake
Copy link
Member

@mathetake mathetake commented May 23, 2022 •
edited
Loading

This implements various SIMD instructions related to
load, store, and lane manipulations for all engines.

Notably, now our engines pass the following specification tests:

  • simd_address.wast
  • simd_const.wast
  • simd_align.wast
  • simd_laod16_lane.wast
  • simd_laod32_lane.wast
  • simd_laod64_lane.wast
  • simd_laod8_lane.wast
  • simd_lane.wast
  • simd_load_extend.wast
  • simd_load_splat.wast
  • simd_load_zero.wast
  • simd_store.wast
  • simd_store16_lane.wast
  • simd_store32_lane.wast
  • simd_store64_lane.wast
  • simd_store8_lane.wast

part of #484

@mathetake mathetake changed the title Implements v128 load an stores Implements v128 load and store instructions May 23, 2022
@mathetake
Copy link
Member Author

as the tests are mutual dependant, so I will repurpose this to support for all load, store and lane manipulation instructions

@mathetake mathetake changed the title Implements v128 load and store instructions SIMD: implements v128 load, store and lane manipulations. May 24, 2022
@mathetake
Copy link
Member Author

ok finished with interpreter... now start working on AOT compilers

@mathetake
Copy link
Member Author

ok the left for amd64 is `v128.shuffle, swizzle, any_true and all_true.... already 4k loc :D

@mathetake
Copy link
Member Author

woohoo finally passed all tests on amd64 👯

@mathetake mathetake force-pushed the v128loadstore branch 2 times, most recently from 70fdb0d to 5317ee8 Compare May 27, 2022 08:29
@mathetake
Copy link
Member Author

The left is all_true, any_true, shuffle and swizzle on arm64...

@mathetake
Copy link
Member Author

shuffle and swizzle...

@mathetake
Copy link
Member Author

finally come to the state where I will backfill the docs and then mark this ready! 8k pure addition 🤯

@mathetake
init
e315770 
Signed-off-by: Takeshi Yoneda <[email protected]>
@mathetake
rm
dc9e5d0 
Signed-off-by: Takeshi Yoneda <[email protected]>
@mathetake mathetake marked this pull request as ready for review May 31, 2022 06:20
@mathetake mathetake requested a review from anuraaga May 31, 2022 06:21
@codefromthecrypt
Copy link
Contributor

I'm doing a typo/link sweep commit, so no one else needs to make comments until I'm done (will save a flurry)

Adrian Cole added 2 commits May 31, 2022 15:53
editorial stuff
9c2ceb2 
Signed-off-by: Adrian Cole <[email protected]>
impl_staticconst.go
145f020 
Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt
codefromthecrypt reviewed May 31, 2022
View reviewed changes
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

next comments

internal/asm/amd64/impl_staticconst.go Outdated Show resolved Hide resolved
internal/asm/amd64/impl_staticconst.go Show resolved Hide resolved
internal/asm/amd64/impl_staticconst.go Outdated Show resolved Hide resolved
internal/asm/amd64/impl_staticconst.go Outdated Show resolved Hide resolved
internal/asm/amd64/impl_staticconst_test.go Show resolved Hide resolved
internal/asm/amd64/impl_staticconst_test.go Outdated Show resolved Hide resolved
internal/asm/amd64/impl_staticconst_test.go Outdated Show resolved Hide resolved
done arm64
faeb274 
Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt
codefromthecrypt reviewed May 31, 2022
View reviewed changes
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

done arm64

internal/asm/arm64/consts.go Show resolved Hide resolved
whoops
d7f4a13 
Signed-off-by: Adrian Cole <[email protected]>
more
4ea8c17 
Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt
codefromthecrypt approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

Great work. I think you left a couple TODOs on some tests to follow-up on, and here are the things I noticed besides what's already commented:

  • impl_vec_*.go are light on overview docs. If you have any links on the approach used to
    implement, please add them where relevant. We can see what's going on, but not why.
  • interpreter needs some summary doc on vector also, just to prepare the reader for what will follow.
  • func_validation needs some test coverage, as does compiler_test, at least hitting the main points

Epic work and thanks for bearing through it!

internal/engine/compiler/compiler_numeric_test.go Show resolved Hide resolved
internal/engine/compiler/compiler_stack_test.go Outdated Show resolved Hide resolved
mathetake added 4 commits June 1, 2022 08:35
@mathetake
apply
8efa4ce 
Signed-off-by: Takeshi Yoneda <[email protected]>
@mathetake
more
eed50b0 
Signed-off-by: Takeshi Yoneda <[email protected]>
@mathetake
unnecessary
b96569e 
Signed-off-by: Takeshi Yoneda <[email protected]>
@mathetake
prefix
90969f8 
Signed-off-by: Takeshi Yoneda <[email protected]>
@mathetake
Copy link
Member Author

will do the follow-up PR soon as this is already huge :D Thank you for your help!

@mathetake mathetake merged commit 0c30325 into main Jun 1, 2022
@mathetake mathetake deleted the v128loadstore branch June 1, 2022 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codefromthecrypt codefromthecrypt codefromthecrypt approved these changes

@anuraaga anuraaga Awaiting requested review from anuraaga

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mathetake @codefromthecrypt