8296447: RISC-V: Make the operands order of vrsub_vx/vrsub_vi consistent with RVV 1.0 spec

[DingliZhang]

Hi,

At the moment, the operands order of vrsub_vx and vrsub_vi is not the same as in the RVV1.0 spec[1]. These instructions use the wrong assembly syntax pattern for vector binary arithmetic instructions (multiply-add)[2].

vrsub_vx was classified as Vector Single-Width Integer Add and Subtract in rvv1.0 spec, but is currently classified as Vector Single-Width Integer Multiply-Add Instructions and generate the functions under the corresponding macros, which results in the reverse order of the operands Vs2 and Rs1 compared to the spec.

vrsub_vi has its own separate macro definition to generate the corresponding function and the order of these operands(Vs2 and imm) is reversed too.

I think it is better to adjust the operands order of these two instructions to be consistent with the spec.

Please take a look and have some reviews. Thanks a lot.

[1] https://github.com/riscv/riscv-v-spec/blob/v1.0/v-spec.adoc
[2] https://github.com/riscv/riscv-v-spec/blob/v1.0/v-spec.adoc#101-vector-arithmetic-instruction-encoding

Testing:

• hotspot and jdk tier1 on unmatched board without new failures
• test/jdk/jdk/incubator/vector/Int256VectorTests.java with fastdebug on qemu
• test/jdk/jdk/incubator/vector/Long256VectorTests.java with fastdebug on qemu

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8296447: RISC-V: Make the operands order of vrsub_vx/vrsub_vi consistent with RVV 1.0 spec

Reviewers

• Ludovic Henry (@luhenry - Author) ⚠️ Review applies to 0930a669
• Fei Yang (@RealFYang - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/11009/head:pull/11009
$ git checkout pull/11009

Update a local copy of the PR:
$ git checkout pull/11009
$ git pull https://git.openjdk.org/jdk pull/11009/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11009

View PR using the GUI difftool:
$ git pr show -t 11009

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/11009.diff

[bridgekeeper]

👋 Welcome back dzhang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@DingliZhang The following label will be automatically applied to this pull request:

• hotspot

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.

[mlbridge]

Webrevs

• 02: Full - Incremental (adeb8e21)
• 01: Full - Incremental (0930a669)
• 00: Full (de67289b)

[luhenry]

I also verified it matches with https://github.com/riscv/riscv-opcodes/blob/master/rv_v

[RealFYang]

Looks fine. Thanks.

[openjdk]

@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:

8296447: RISC-V: Make the operands order of vrsub_vx/vrsub_vi consistent with RVV 1.0 spec

Reviewed-by: luhenry, fyang

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 18 new commits pushed to the master branch:

• 4c80dff: 8296435: RISC-V: Small refactoring for increment/decrement
• 47d2c7b: 8295376: Improve debug agent virtual thread performance when no debugger is attached
• 76790ad: 8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders
• b6738c1: 8295663: Rephrase introduction to testing.md
• 7e85b41: 8296154: [macos] Change "/Applications" to "Applications" in DMG image
• 60db5f2: 8294020: improve errors for record declarations
• 520db1e: 8296485: BuildEEBasicConstraints.java test fails with SunCertPathBuilderException
• b29817a: 8296479: Remove stray comment about POST_STRIP_CMD
• b6ea69c: 8296480: java/security/cert/pkix/policyChanges/TestPolicy.java is failing
• 3baad06: 8295953: Use enhanced-for cycle instead of Enumeration in sun.security
• ... and 8 more: https://git.openjdk.org/jdk/compare/c2f76383895e3d054988a5817de52e7795bf69c2...master

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 (@RealFYang) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[DingliZhang]

@luhenry @RealFYang Thanks for the review!
/integrate

[openjdk]

@DingliZhang
Your change (at version adeb8e2) is now ready to be sponsored by a Committer.

[RealFYang]

/sponsor

[openjdk]

Going to push as commit 1169dc0.
Since your change was applied there have been 18 commits pushed to the master branch:

• 4c80dff: 8296435: RISC-V: Small refactoring for increment/decrement
• 47d2c7b: 8295376: Improve debug agent virtual thread performance when no debugger is attached
• 76790ad: 8295673: Deprecate and disable legacy parallel class loading workaround for non-parallel-capable class loaders
• b6738c1: 8295663: Rephrase introduction to testing.md
• 7e85b41: 8296154: [macos] Change "/Applications" to "Applications" in DMG image
• 60db5f2: 8294020: improve errors for record declarations
• 520db1e: 8296485: BuildEEBasicConstraints.java test fails with SunCertPathBuilderException
• b29817a: 8296479: Remove stray comment about POST_STRIP_CMD
• b6ea69c: 8296480: java/security/cert/pkix/policyChanges/TestPolicy.java is failing
• 3baad06: 8295953: Use enhanced-for cycle instead of Enumeration in sun.security
• ... and 8 more: https://git.openjdk.org/jdk/compare/c2f76383895e3d054988a5817de52e7795bf69c2...master

Your commit was automatically rebased without conflicts.

[openjdk]

@RealFYang @DingliZhang Pushed as commit 1169dc0.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.