RFR: 8258953: AArch64: move NEON instructions to aarch64_neon.ad

[Ningsheng Jian]

On Thu, 28 Jan 2021 01:37:33 GMT, Dong Bo <dongbo at openjdk.org> wrote:

> As discussed in [1], all NEON instructions should be moved from `aarch64.ad` to `aarch64_neon.ad`.
> 
> In the first commit [2] of this PR, the NEON instructions are deleted from `aarch64.ad` and appended to `aarch64_neon.ad`.
> I compared the generated code in `aarch64_neon.ad` with original code in `aarch64.ad`, no suspicious differences found.
> The last two commits just simply move code around in `aarch64_neon.ad` to put related instructions together, i.e. `LoadStore` [3], `Reduction` [4].
> 
> This also supports vector length 4 for `vsraa8B_imm` and `vsrla8B_imm`, vector length 2 for `vsraa4S_imm` and `vsrla4S_imm`, fixes few typos, e.g. `vor8B`, `vsrla4S_imm`.
> 
> [1] https://github.com/openjdk/jdk/pull/1215#issuecomment-728186803
> [2] https://github.com/dgbo/jdk/commit/40cbe99e647cdf93712edf8f77ab3b5b30ea0a95
> [3] https://github.com/dgbo/jdk/commit/695fb8f8ef009b733a8f804e791347f4bfe2572e
> [4] https://github.com/dgbo/jdk/commit/e0c38aa9aaa6af9925a3821328384b1e2b2c2070

I managed to sort all the instructs and compare them with and without the patch. They are general the same except for some trailing whitespaces and typos you mentioned.

src/hotspot/cpu/aarch64/aarch64_neon.ad line 776:

> 774:            as_FloatRegister($vsrc$$reg), 0, 1);
> 775:     __ fadds(as_FloatRegister($dst$$reg),
> 776:              as_FloatRegister($dst$$reg), as_FloatRegister($tmp$$reg)); 

Remove trailing whitespace.

src/hotspot/cpu/aarch64/aarch64_neon.ad line 880:

> 878:            as_FloatRegister($vsrc$$reg), 0, 1);
> 879:     __ faddd(as_FloatRegister($dst$$reg),
> 880:              as_FloatRegister($dst$$reg), as_FloatRegister($tmp$$reg)); 

Trailing whitespace.

src/hotspot/cpu/aarch64/aarch64_neon.ad line 800:

> 798:            as_FloatRegister($vsrc$$reg), 0, 1);
> 799:     __ fadds(as_FloatRegister($dst$$reg),
> 800:              as_FloatRegister($dst$$reg), as_FloatRegister($tmp$$reg)); 

trailing whitespace

src/hotspot/cpu/aarch64/aarch64_neon.ad line 4936:

> 4934:   format %{ "fsqrt  $dst, $src\t# vector (2D)" %}
> 4935:   ins_encode %{
> 4936:     __ fsqrt(as_FloatRegister($dst$$reg), __ T2D, 

trailing whitespace

src/hotspot/cpu/aarch64/aarch64_neon.ad line 900:

> 898:            as_FloatRegister($vsrc$$reg), 0, 1);
> 899:     __ fmuld(as_FloatRegister($dst$$reg),
> 900:              as_FloatRegister($dst$$reg), as_FloatRegister($tmp$$reg)); 

trailing whitespace?

src/hotspot/cpu/aarch64/aarch64_neon.ad line 5022:

> 5020:   match(Set dst (OrV src1 src2));
> 5021:   ins_cost(INSN_COST);
> 5022:   format %{ "orr  $dst,$src1,$src2\t# vector (8B)" %}

Good catch for this typo!

src/hotspot/cpu/aarch64/aarch64_neon.ad line 5937:

> 5935:              as_FloatRegister($src$$reg));
> 5936:     } else {
> 5937:       __ usra(as_FloatRegister($dst$$reg), __ T4H,

I see you have fixed this typo, from ushr to usra. I presume original version generates wrong code and produces wrong results for specific case? If so, do you think it deserves a separate fix, e.g. for jdk16?

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

PR: https://git.openjdk.java.net/jdk/pull/2273