-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
8307609: RISC-V: Added support for Extract, Compress, Expand and other nodes for Vector API #13862
Conversation
…r nodes for Vector API
👋 Welcome back dzhang! A progress list of the required criteria for merging this PR into |
@DingliZhang The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
/contributor add zifeihan caogui@iscas.ac.cn |
@DingliZhang |
Webrevs
|
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.
Some initial comments from a cursory look.
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.
Overall looks good, with some comments.
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.
Looks good, thanks.
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.
Thanks for the update. Would you mind a few more tweaks?
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.
Some extra nit-picking suggestions. Otherwise, looks good. Thanks.
@DingliZhang This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@RealFYang) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
@feilongjiang @RealFYang Thanks for the review! |
/sponsor |
@DingliZhang |
Going to push as commit 97ade57. |
@RealFYang @DingliZhang Pushed as commit 97ade57. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Hi all,
We have added support for Extract, Compress, Expand and other nodes for Vector
API. It was implemented by referring to RVV v1.0 [1]. Please take a look and
have some reviews. Thanks a lot.
In this PR, we will support these new nodes:
At the same time, we refactored methods such as
match_rule_supported_vector_mask
. All implemented vector nodes support maskoperations by default now, so we also added mask nodes for all implemented
nodes.
By the way, we will implement the VectorTest node in the next PR.
We can use the tests under
test/jdk/jdk/incubator/vector
to print thecompilation log for most of the new nodes. And we can use the following
command to print the compilation log of a jtreg test case:
CompressM/CompressV/ExpandV
There is no inverse vdecompress provided in RVV, as this operation can be
readily synthesized using iota and a masked vrgather in
ExpandV
.We can use
test/jdk/jdk/incubator/vector/Float256VectorTests.java
to emitthese nodes and the compilation log is as follows:
LoadVectorGather/StoreVectorScatter/LoadVectorGatherMasked/StoreVectorScatterMasked
We use the vsoxei32_v instruction regardless of what sew is set to. The
indexMap in fromArray is an int array, so the index is always 32 bits. Because
index stores the index value, and vs2 of vsoxei32_v requires an offset, we need
to multiply the value corresponding to idx by the number of bytes of data width.
We can use
test/jdk/jdk/incubator/vector/Float256VectorLoadStoreTests.java
toemit these nodes and the compilation log is as follows:
Extract
Extract is used to return the element from a vector with the given index.
We can use
test/jdk/jdk/incubator/vector/*MaxVectorTests.java
to emit thesenodes and the compilation log is as follows:
AndV/OrV/XorV masked
We can use
Byte128VectorTests.java
to emit these nodes and the compilationlog is as follows:
VectorLongToMask/VectorMaskToLong
We can use
VectorMaskLoadStoreTest.java
andFloat256VectorTests.java
toemit these nodes and the compilation log is as follows:
PopulateIndex
We need
PopulateIndexNode
to enable the vectorization of operations with loopinduction variable by extending current scope of C2 superword vectorizable
packs, just like JDK-8280510.
With this we can vectorize some operations in loop with the induction variable
operand, such as below.
Final compilation log for above loop expression is like below.
Hotspot jtreg has existing tests in
compiler/c2/cr7192963/Test*Vect.java
andwill be all passed.
VectorLongToMask/VectorMaskToLong
We can use
VectorMaskLoadStoreTest.java
andFloat256VectorTests.java
toemit these nodes and the compilation log is as follows:
VectorMaskTrueCount/VectorMaskFirstTrue
We can use
Double128VectorTests.java
to emit these nodes and the compilationlog is as follows:
VectorInsert
We can use
test/hotspot/jtreg/compiler/vectorapi/TestVectorInsertByte.java
toemit lt32 node and the compilation log is as follows:
In order to cover the case where idx is greater than 31, we need to modify
TestVectorInsertByte.java
:Then the compilation log is as follows:
MaskAll masked
SVE can use the case
shuffleTest()
inInt64VectorTests.java
to emitvmaskAllI_masked, and the function
vector_needs_partial_operations
willjudge and emit masked vmaskAllI node. RISC-V uses vsetvl to set vector element
length, so we do not need partial operations. But we can use
vector_needs_partial_operations
to cover vmaskAllI_masked this point.Apply patch:
Then the compilation log is as follows:
[1] https://github.com/riscv/riscv-v-spec/blob/v1.0/v-spec.adoc
Testing:
qemu with UseRVV:
Progress
Issue
Reviewers
Contributors
<caogui@iscas.ac.cn>
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13862/head:pull/13862
$ git checkout pull/13862
Update a local copy of the PR:
$ git checkout pull/13862
$ git pull https://git.openjdk.org/jdk.git pull/13862/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 13862
View PR using the GUI difftool:
$ git pr show -t 13862
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13862.diff
Webrev
Link to Webrev Comment