-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8355074: RISC-V: C2: Support Vector-Scalar version of Zvbb Vector And-Not instruction #24709
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
Conversation
|
👋 Welcome back Anjian-Wen! A progress list of the required criteria for merging this PR into |
|
@Anjian-Wen 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 6 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. 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 (@feilongjiang, @RealFYang) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
|
@Anjian-Wen 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. |
|
@Anjian-Wen this pull request can not be integrated into git checkout Vector_And-Not_vx
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
Support Zvbb Vector And-Not vx and add its tests modify name of the match rule and test name
ae63f14 to
5617982
Compare
|
/issue JDK-8355074 |
|
@Anjian-Wen The primary solved issue for a PR is set through the PR title. Since the current title does not contain an issue reference, it will now be updated. |
Webrevs
|
| machOnlyNameRegex(VAND_NOT_L_MASKED, "vand_notL_masked"); | ||
| } | ||
|
|
||
| public static final String VAND_NOTI_VX = PREFIX + "VAND_NOTI_VX" + POSTFIX; |
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.
Since this is a shared file and these four instructions are RISCV-specific, seems better to add a RISCV_ prefix to make that explicit. Consider names like RISCV_VAND_NOTI_VX, RISCV_VAND_NOTL_VX, RISCV_VAND_NOTI_VX_MASKED and RISCV_VAND_NOTL_VX_MASKED.
feilongjiang
left a comment
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! Overall looks good, with one minor suggestion.
src/hotspot/cpu/riscv/riscv_v.ad
Outdated
| // vector-immediate add (unpredicated) | ||
|
|
||
| instruct vadd_immI(vReg dst, vReg src1, immI5 con) %{ | ||
| instruct vaddI_vi(vReg dst, vReg src1, immI5 con) %{ |
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.
Perhaps we can do these naming refactorings in a separate PR first. This will look cleaner.
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, that's a good idea, I'll split it into two patch
|
here is the another related pr for clean up |
RealFYang
left a comment
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.
Updated change looks fine. Thanks.
|
/integrate |
|
@Anjian-Wen |
|
/sponsor |
|
Going to push as commit 5c06723.
Your commit was automatically rebased without conflicts. |
|
@RealFYang @Anjian-Wen Pushed as commit 5c06723. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
support zvbb vand-not vector-scalar version, which Op1 is the sign-extended or truncated value in scalar register rs1
add C2 match rule
add related Tests in IRNode structure
passed jtreg test test/hotspot/jtreg/compiler/vectorapi/*
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24709/head:pull/24709$ git checkout pull/24709Update a local copy of the PR:
$ git checkout pull/24709$ git pull https://git.openjdk.org/jdk.git pull/24709/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 24709View PR using the GUI difftool:
$ git pr show -t 24709Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24709.diff
Using Webrev
Link to Webrev Comment