RFR: 8309332: RISC-V: Improve PrintOptoAssembly output of vector nodes [v2]

Fri Jun 2 09:42:21 UTC 2023

On Fri, 2 Jun 2023 02:58:06 GMT, Gui Cao <gcao at openjdk.org> wrote:

>> Hi, Currently in the vector node implementation, some of the instruction formats are part of the rvv assembly instructions, for which the assembly instructions are also incomplete, such as the missing vsetvli assembly instruction(vsetvli is used to set the element width, number, etc), This makes the compiler assembly output by -XX:+PrintOptoAssembly difficult to understand. And if every assembly instruction is reflected, it will be redundant and increase the maintenance work in the future. And referring to other CPUs, such as the ARM64, it is straightforward to use the instruct function name to simplify [1].
>> 
>> While this won't affect release build, we should fix this for debug build. We can use the -XX:+PrintOptoAssembly parameter to print the compiler assembly output by -XX:+PrintOptoAssembly and view the assembly logic in conjunction with the source code of the specific vector node.
>> 
>> [1] https://github.com/openjdk/jdk/blob/4460429d7a50b9a7a99058ef4e5ae36fb30b956f/src/hotspot/cpu/aarch64/aarch64_vector.ad#L2842-L2846
>> 
>> ### Testing:
>> qemu with UseRVV:
>> 
>> - [ ] Tier1 tests (release)
>> - [x] test/jdk/jdk/incubator/vector (fastdebug)
>> - [x] test/jdk/jdk/incubator/vector/Int256VectorTests.java (fastdebug with -XX:+PrintOptoAssembly)
>
> Gui Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update vneg/vfneg instruct format

Changes requested by fyang (Reviewer).

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

> 342:   match(Set dst_src1 (AddVL (Binary dst_src1 src2) v0));
> 343:   ins_cost(VEC_COST);
> 344:   format %{ "vadd_masked $dst_src1, $src2, $v0" %}

Suggestion: `format %{ "vadd_masked $dst_src1, $dst_src1, $src2, $v0" %}`

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

> 357:   match(Set dst_src1 (AddVD (Binary dst_src1 src2) v0));
> 358:   ins_cost(VEC_COST);
> 359:   format %{ "vadd_fp_masked $dst_src1, $src2, $v0" %}

Suggestion: `format %{ "vadd_fp_masked $dst_src1, $dst_src1, $src2, $v0" %}`

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

> 408:   match(Set dst_src1 (SubVL (Binary dst_src1 src2) v0));
> 409:   ins_cost(VEC_COST);
> 410:   format %{ "vsub_masked $dst_src1, $src2, $v0" %}

Suggestion: `format %{ "vsub_masked $dst_src1, $dst_src1, $src2, $v0" %}`

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

> 422:   match(Set dst_src1 (SubVD (Binary dst_src1 src2) v0));
> 423:   ins_cost(VEC_COST);
> 424:   format %{ "vsub_fp_masked $dst_src1, $src2, $v0" %}

Suggestion: `format %{ "vsub_fp_masked $dst_src1, $dst_src1, $src2, $v0" %}`

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

> 551:   match(Set dst_src1 (DivVD (Binary dst_src1 src2) v0));
> 552:   ins_cost(VEC_COST);
> 553:   format %{ "vdiv_fp_masked $dst_src1, $src2, $v0" %}

Suggestion: `format %{ "vdiv_fp_masked $dst_src1, $dst_src1, $src2, $v0" %}`

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

> 790:   match(Set dst_src1 (FmaVF dst_src1 (Binary src2 (NegVF src3))));
> 791:   ins_cost(VEC_COST);
> 792:   format %{ "vfmlsF $dst_src1, $src2, $src3" %}

Suggestion: `format %{ "vfmlsF $dst_src1, $dst_src1, $src2, $src3" %}`

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

> 806:   match(Set dst_src1 (FmaVD dst_src1 (Binary src2 (NegVD src3))));
> 807:   ins_cost(VEC_COST);
> 808:   format %{ "vfmlsD $dst_src1, $src2, $src3" %}

Similar here.

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

> 841:   match(Set dst_src1 (FmaVF (NegVF dst_src1) (Binary src2 (NegVF src3))));
> 842:   ins_cost(VEC_COST);
> 843:   format %{ "vfnmlaF $dst_src1, $src2, $src3" %}

Similar here.

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

> 857:   match(Set dst_src1 (FmaVD (NegVD dst_src1) (Binary src2 (NegVD src3))));
> 858:   ins_cost(VEC_COST);
> 859:   format %{ "vfnmlaD $dst_src1, $src2, $src3" %}

Similar here.

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

> 890:   match(Set dst_src1 (FmaVF (NegVF dst_src1) (Binary src2 src3)));
> 891:   ins_cost(VEC_COST);
> 892:   format %{ "vfnmlsF $dst_src1, $src2, $src3" %}

Similar here.

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

> 904:   match(Set dst_src1 (FmaVD (NegVD dst_src1) (Binary src2 src3)));
> 905:   ins_cost(VEC_COST);
> 906:   format %{ "vfnmlsD $dst_src1, $src2, $src3" %}

Similar here.

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

> 939:   match(Set dst_src1 (AddVL dst_src1 (MulVL src2 src3)));
> 940:   ins_cost(VEC_COST);
> 941:   format %{ "vmla $dst_src1, src2, src3" %}

Similar here.

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

> 975:   match(Set dst_src1 (SubVL dst_src1 (MulVL src2 src3)));
> 976:   ins_cost(VEC_COST);
> 977:   format %{ "vmls $dst_src1, src2, src3" %}

Similar here.

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

> 1042:   match(Set dst_src1 (MulVL (Binary dst_src1 src2) v0));
> 1043:   ins_cost(VEC_COST);
> 1044:   format %{ "vmul_masked $dst_src1, $src2, $v0" %}

Similar here.

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

> 1056:   match(Set dst_src1 (MulVD (Binary dst_src1 src2) v0));
> 1057:   ins_cost(VEC_COST);
> 1058:   format %{ "vmul_fp_masked $dst_src1, $src2, $v0" %}

Similar here.

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

> 1376:   effect(TEMP tmp);
> 1377:   ins_cost(VEC_COST);
> 1378:   format %{ "reduce_addF $src1_dst, $src2\t# KILL $tmp" %}

Similar here.

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

> 1391:   effect(TEMP tmp);
> 1392:   ins_cost(VEC_COST);
> 1393:   format %{ "reduce_addD $src1_dst, $src2\t# KILL $tmp" %}

Similar here.

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

PR Review: https://git.openjdk.org/jdk/pull/14279#pullrequestreview-1456996124
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214146356
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214146867
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214147354
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214147929
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214148773
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214149484
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214150847
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214150931
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151243
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151369
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151467
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151581
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151646
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151780
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214151919
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214153194
PR Review Comment: https://git.openjdk.org/jdk/pull/14279#discussion_r1214153344