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