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

Implement 256 and 512-bit vload/vstore on x86 #6509

Merged
merged 1 commit into from
May 16, 2022
Merged

Implement 256 and 512-bit vload/vstore on x86 #6509

merged 1 commit into from
May 16, 2022

Conversation

BradleyWood
Copy link
Member

@BradleyWood BradleyWood commented May 11, 2022 •
edited
Loading

This commit enables 256 and 512 bit vector lengths
on x86 hardware. As a precursor to supporting new
vector opcodes at these vector lengths, this commit
supports vector loads and stores of 256 and 512
bit vectors by choosing the correct instruction
encoding form depending on the vector length.
A parameterized tril test is added to test the
vload/vstore implementations at all vector lengths
and for all element types.

@BradleyWood
Copy link
Member Author

BradleyWood commented May 11, 2022 •
edited
Loading

Jenkins build all

@BradleyWood
Copy link
Member Author

Jenkins build all

@BradleyWood
Copy link
Member Author

Jenkins build all

@BradleyWood
Copy link
Member Author

Jenkins build all

@BradleyWood BradleyWood marked this pull request as ready for review May 12, 2022 03:24
@BradleyWood
Copy link
Member Author

@jdmpapin Could you review and process this PR.

@BradleyWood BradleyWood mentioned this pull request May 12, 2022
Merged
@gita-omr
Copy link
Contributor

LGTM

@jdmpapin jdmpapin self-assigned this May 13, 2022
jdmpapin
jdmpapin reviewed May 13, 2022
View reviewed changes
compiler/x/codegen/OMRCodeGenerator.cpp Outdated Show resolved Hide resolved
compiler/x/codegen/OMRCodeGenerator.cpp Outdated Show resolved Hide resolved
fvtest/compilertriltest/VectorTest.cpp Outdated Show resolved Hide resolved
fvtest/compilertriltest/VectorTest.cpp Show resolved Hide resolved
fvtest/compilertriltest/VectorTest.cpp Outdated Show resolved Hide resolved
compiler/x/codegen/SIMDTreeEvaluator.cpp Outdated Show resolved Hide resolved
@BradleyWood
Support 256/512-bit vector load and store operations
eccfe2c 
This commit enables 256 and 512 bit vector lengths
on x86 hardware. As a precursor to supporting new
vector opcodes at these vector lengths, this commit
supports vector loads and stores of 256 and 512
bit vectors by choosing the correct instruction
encoding form depending on the vector length.
A parameterized tril test is added to test the
vload/vstore implementations at all vector lengths
and for all element types.

Signed-off-by: BradleyWood <[email protected]>
@jdmpapin
Copy link
Contributor

Jenkins build all

@jdmpapin
Copy link
Contributor

Jenkins build osx

@jdmpapin
Copy link
Contributor

Jenkins build zos

1 similar comment
@jdmpapin
Copy link
Contributor

Jenkins build zos

@jdmpapin
Copy link
Contributor

Originally a couple of tests failed on Mac, but the failure was definitely unrelated to these changes because they were port library tests. The same tests have also failed in other PRs, and the problem is now tracked as #6516

@jdmpapin jdmpapin merged commit 60ff7f2 into eclipse:master May 16, 2022
@pshipton pshipton mentioned this pull request May 17, 2022
Closed
@pshipton
Copy link
Member

@jdmpapin this change is causing a problem in OMR acceptance for OpenJ9, eclipse-openj9/openj9#15075

Unless there is somebody who can fix it, we may want to revert to unblock OMR acceptance.

@jdmpapin
Copy link
Contributor

Reverting seems reasonable to me

@pshipton pshipton mentioned this pull request May 17, 2022
Merged
@pshipton
Copy link
Member

I used the revert button and created #6521

@BradleyWood
Copy link
Member Author

The failure seems to be intermittent on my machine. Originally, vload and vstore were not supported for int8 and int16 element types. I had removed removed that check because the element type of the copy shouldn't matter. For some reason removing that check causes cmdLineTester_loopReduction_0 to fail.

@BradleyWood
Copy link
Member Author

BradleyWood commented May 26, 2022 •
edited
Loading

The issue seems to be a result of an autoSIMD bug. I observed vsplatsVector128Int8 opcode in a trace file even though getSupportsOpCodeForAutoSIMD() returns false for vsplats at int8 element type because its not implemented. At some point, vsplatsVector128Int8 is generated if vstoreiVector128Int8 is supported, without checking if the vsplats opcode is supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdmpapin jdmpapin jdmpapin approved these changes

@Leonardo2718 Leonardo2718 Awaiting requested review from Leonardo2718

@fjeremic fjeremic Awaiting requested review from fjeremic fjeremic is a code owner

@0xdaryl 0xdaryl Awaiting requested review from 0xdaryl 0xdaryl is a code owner

@vijaysun-omr vijaysun-omr Awaiting requested review from vijaysun-omr vijaysun-omr is a code owner

@mstoodle mstoodle Awaiting requested review from mstoodle mstoodle is a code owner

Assignees

@jdmpapin jdmpapin

Labels
arch:aarch64 arch:power arch:x86 arch:z comp:compiler comp:test comp:tril documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BradleyWood @gita-omr @jdmpapin @pshipton