RFR: 8332153: RISC-V: enable tests and add comment for vector shift instruct (shared by vectorization and Vector API) [v2]

Fei Yang fyang at openjdk.org
Mon May 20 06:43:06 UTC 2024


On Thu, 16 May 2024 14:45:16 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review this patch?
>> For vector shift instruct, some corresponding tests are not enabled, this is to enable them.
>> And the way how vector shift instruct works is not clear, especially both vectorization (SLP in jdk) and Vector API share the same instruct's in riscv_v.ad, so also added some comment to clarify it.
>> 
>> Thanks
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix words

src/hotspot/cpu/riscv/riscv_v.ad line 1796:

> 1794: // check https://docs.oracle.com/javase/specs/jls/se21/html/jls-15.html#jls-15.19 for details.
> 1795: //
> 1796: // Shift behaviour in Vector APi is defined as:

Nit: s/APi/API/

test/hotspot/jtreg/compiler/c2/aarch64/TestVectorShiftShorts.java line 31:

> 29:  *
> 30:  * @requires vm.compiler2.enabled
> 31:  * @requires os.arch == "aarch64" | os.arch == "riscv64"

Seems a bit weird to enable tests under `test/hotspot/jtreg/compiler/c2/aarch64/` for other cpus?

test/hotspot/jtreg/compiler/vectorization/runner/ArrayShiftOpTest.java line 103:

> 101:         counts = {IRNode.RSHIFT_VI, ">0"})
> 102:     @IR(applyIfPlatform = {"riscv64", "true"},
> 103:         applyIfCPUFeature = {" v ", "true"},

Is the spaces around `v` necessary?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19265#discussion_r1606309014
PR Review Comment: https://git.openjdk.org/jdk/pull/19265#discussion_r1606318534
PR Review Comment: https://git.openjdk.org/jdk/pull/19265#discussion_r1606318107


More information about the hotspot-compiler-dev mailing list