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

Dong Bo dongbo at openjdk.java.net
Thu Jan 28 12:12:45 UTC 2021


On Thu, 28 Jan 2021 10:37:47 GMT, Ningsheng Jian <njian 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.

> _Mailing list message from [Andrew Haley](mailto:aph at redhat.com) on [hotspot-dev](mailto:hotspot-dev at openjdk.java.net):_
> 
> On 1/28/21 10:40 AM, Ningsheng Jian wrote:
> 
> > 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?
> 
> It does. This patch should change nothing at all, except moving
> text from A to B.
> 
> --
> Andrew Haley (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

@nsjian @theRealAph Thank you for the comments. I'll raise a seperate PR to fix this right now.

BTW, since Andrew says we should change nothing at all in this move, do you think we should also do the things below in separtate PRs?
1. fix the typo of `vor8B`.
2. supporting vector length 4 for `vsraa8B_imm` and `vsrla8B_imm`, vector length 2 for `vsraa4S_imm` and `vsrla4S_imm`.

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

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


More information about the hotspot-compiler-dev mailing list