-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8329077: C2 SuperWord: Add MoveD2L, MoveL2D, MoveF2I, MoveI2F #26457
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.
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
Open
galderz
wants to merge
22
commits into
openjdk:master
Choose a base branch
from
galderz:topic.fp-bits-vector
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+63
−10
Open
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
f747a0a
floatToRawIntBits: initial impl
galderz 6b1a1eb
floatToRawIntBits: vectorized JMH microbenchmark
galderz 4a86185
floatToRawIntBits: add missing @State in benchmark
galderz 8666ca0
floatToRawIntBits: switch benchmark to float
galderz e15c143
add floatToIntBits benchmark
galderz 61625fe
Support doubleToRawLongBits
galderz d69ecad
Add JMH benchmark for doubleTo*LongBits
galderz 2bace0c
Add basic IR test
galderz 18e8ce2
Fix test for vectorized and add floatToRawIntBits
galderz 25199da
Add longBitsToDouble and intBitsToFloat
galderz cae1f08
Merge branch 'master' into topic.fp-bits-vector
galderz 3e527c2
Delete IR test because it's already covered by other test
galderz 7e0e70b
Adjust IR test after adding Move* vector support
galderz b6ec784
Removed unnecessary assert methods
galderz 1b8babd
Rephrase comment
galderz 879eddb
Refactor and add copyright header
galderz dde8699
Avoid VectorReinterpret::implemented
galderz 147633f
Check at the very least that auto vectorization is supported
galderz cfe998c
Refactor benchmarks to TypeVectorOperations
galderz 89d9bbb
Fix source of data for benchmarks
galderz 01fd5ba
Add more IR node positive assertions
galderz e7e4d80
Merge branch 'master' into topic.fp-bits-vector
galderz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this affect
Op_ReinterpretHF2S
that is also inVectorNode::is_reinterpret_opcode
?I'm afraid that we need to test this with hardware or Intel's SDE, to make sure we have it running on a VM that actually supports Float16. Otherwise these instructions may not be used, and hence not tested right.
@galderz Can you run the relevant tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you run specific tiers in those platforms? Just hotspot compiler? Or individual tests such as
ConvF2HFIdealizationTests
andTestFloat16ScalarOperations
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I don't know, I'd have to do the research myself. Probably focusing on the Float16 tests would be good enough. No other test would really use Float16, so running anything else would not be that useful probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done some testing on x86_64 and aarch64 and the tests pass.
I also made sure that the test output demonstrated execution of the expected IR rule as per the requirements of each platform.
c7gn.2xlarge
Graviton3c7i.xlarge
Intel(R) Xeon(R) Platinum 8488C (saphire rapids):There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I've noticed that
TestFloat16ScalarOperations
does not havepackage
definition. Is that an oversight? It runs fine in spite of not having itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, as you mostly touched the auto-vectorization part of c2, could you please run these float16 tests as well (most of these enable auto-vectorization for Float16) -
compiler/vectorization/TestFloat16VectorOperations.java
compiler/vectorization/TestFloatConversionsVectorNaN.java
compiler/vectorization/TestFloatConversionsVector.java
compiler/vectorization/TestFloat16ToFloatConv.java
compiler/vectorization/TestFloat16VectorConvChain.java
compiler/intrinsics/float16/*
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Bhavana-Kilambi I've run these tests:
On x86:
On graviton 3 aarch64:
The failure on aarch64 is already existing issue JDK-8361582
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@galderz Excellent, that's great :)