Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On 28/09/2020 20:12, Bernhard Urban-Forster wrote:
> The idea is that the naming should suggest that `r18` shouldn't be used on
> that particular platform. Same is true for
> macOS, but the ABI docs suggest a different usage, hence we have something
> like that in our internal branch for macOS:
> Suggestion:
>
> "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls")
> MACOS_ONLY("rplatform"), "r19",
> Are you suggesting it should rather be something like this eventually?
> Suggestion:
>
> "r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
> "r19",
This is wrong. We can't use the register in Linux either, except in
Linux-specific code of which there is almost none. It's also a
pointless complication. r18 should be called r18_tls everywhere, as
discussed.
--
Andrew Haley (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd.
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On 28/09/2020 20:12, Bernhard Urban-Forster wrote:
> The idea is that the naming should suggest that `r18` shouldn't be used on
> that particular platform. Same is true for
> macOS, but the ABI docs suggest a different usage, hence we have something
> like that in our internal branch for macOS:
> Suggestion:
>
> "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls")
> MACOS_ONLY("rplatform"), "r19",
> Are you suggesting it should rather be something like this eventually?
> Suggestion:
>
> "r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
> "r19",
This wrong. We can't use the register in Linux only, except in Linux-specific
code of which there is almost none. It should be called r18_tls everywhere,
as discussed.
--
Andrew Haley (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd.
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 19:38:15 GMT, Bernhard Urban-Forster
wrote:
>> this looks better I think, if it's done right from beginning, we won't have
>> to modify it later.
>> The Question is, can we do it ahead of JEP-391 ?
>> If we can't then maybe better to leave it this way for now:
>> WIN64_ONLY("rtls") NOT_WIN64("r18")
>>
>> as r18 is supposed to be reserved register on aarch64, linux is just an
>> exception where it's allowed.
>
> Let us go with the following in this PR as that's the extend of this PR:
> Suggestion:
>
> "r17", LINUX_ONLY("r18") WIN64_ONLY("rtls"), "r19",
> We can update it accordingly in a PR for JEP-391.
Ok, Great.
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 19:28:10 GMT, Vladimir Kempik wrote:
>> The idea is that the naming should suggest that `r18` shouldn't be used on
>> that particular platform. Same is true for
>> macOS, but the ABI docs suggest a different usage, hence we have something
>> like that in our internal branch for macOS:
>> Suggestion:
>> "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls")
>> MACOS_ONLY("rplatform"), "r19",
>> Are you suggesting it should rather be something like this eventually?
>> Suggestion:
>>
>> "r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
>> "r19",
>
> this looks better I think, if it's done right from beginning, we won't have
> to modify it later.
> The Question is, can we do it ahead of JEP-391 ?
> If we can't then maybe better to leave it this way for now:
> WIN64_ONLY("rtls") NOT_WIN64("r18")
>
> as r18 is supposed to be reserved register on aarch64, linux is just an
> exception where it's allowed.
Let us go with the following in this PR as that's the extend of this PR:
Suggestion:
"r17", LINUX_ONLY("r18") WIN64_ONLY("rtls"), "r19",
We can update it accordingly in a PR for JEP-391.
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 19:09:17 GMT, Bernhard Urban-Forster
wrote:
>> src/hotspot/cpu/aarch64/register_aarch64.cpp line 44:
>>
>>> 42: "rscratch1", "rscratch2",
>>> 43: "r10", "r11", "r12", "r13", "r14", "r15", "r16",
>>> 44: "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls"), "r19",
>>
>> For me this line doesn't look good in case of expanding this functionality
>> to macos-aarch64
>> as it's just the name of register and it's r18 on every platform except
>> WIN64 and has nothing to do with reserving r18.
>
> The idea is that the naming should suggest that `r18` shouldn't be used on
> that particular platform. Same is true for
> macOS, but the ABI docs suggest a different usage, hence we have something
> like that in our internal branch for macOS:
> Suggestion:
> "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls")
> MACOS_ONLY("rplatform"), "r19",
> Are you suggesting it should rather be something like this eventually?
> Suggestion:
>
> "r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
> "r19",
this looks better I think, if it's done right from beginning, we won't have to
modify it later.
The Question is, can we do it ahead of JEP-391 ?
If we can't then maybe better to leave it this way for now:
WIN64_ONLY("rtls") NOT_WIN64("r18")
as r18 is supposed to be reserved register on aarch64, linux is just an
exception where it's allowed.
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 17:37:32 GMT, Vladimir Kempik wrote:
>> Monica Beckwith has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now
>> contains 24 commits:
>> - Merge remote-tracking branch 'upstream/master' into jdk-windows
>> - SA: update copyright
>> - Fix graal codestyle
>> - Reduce includes
>> - Merge remote-tracking branch 'upstream/master' into jdk-windows
>> - os_windows: remove duplicated UMA handling
>> - test_safefetch{32,N} works fine on win+aarch64
>> - cleanup for 8253539: Remove unused JavaThread functions for
>> set_last_Java_fp/pc
>> - cleanup for 8253457: Remove unimplemented register stack functions
>> - Merge remote-tracking branch 'upstream/master' into jdk-windows
>> - ... and 14 more:
>> https://git.openjdk.java.net/jdk/compare/ec9bee68...a7cdaad6
>
> src/hotspot/cpu/aarch64/register_aarch64.cpp line 44:
>
>> 42: "rscratch1", "rscratch2",
>> 43: "r10", "r11", "r12", "r13", "r14", "r15", "r16",
>> 44: "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls"), "r19",
>
> For me this line doesn't look good in case of expanding this functionality to
> macos-aarch64
> as it's just the name of register and it's r18 on every platform except WIN64
> and has nothing to do with reserving r18.
The idea is that the naming should suggest that `r18` shouldn't be used on that
particular platform. Same is true for
macOS, but the ABI docs suggest a different usage, hence we have something like
that in our internal branch for macOS:
Suggestion:
"r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
"r19",
Are you suggesting it should rather be something like this eventually?
Suggestion:
"r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"), "r19",
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 14:07:16 GMT, Monica Beckwith wrote:
>> This is a continuation of
>> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>>
>> Changes since then:
>> * We've improved the write barrier as suggested by Andrew [1]
>> * The define-guards around R18 have been changed to `R18_RESERVED`. This
>> will be enabled for Windows only for now but
>> will be required for the upcoming macOS+Aarch64 [2] port as well.
>> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov
>> in our PR for now and built the
>> Windows-specific CPU feature detection on top of it.
>>
>> [1]
>> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html
>> [2] https://openjdk.java.net/jeps/8251280
>
> Monica Beckwith has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now
> contains 24 commits:
> - Merge remote-tracking branch 'upstream/master' into jdk-windows
> - SA: update copyright
> - Fix graal codestyle
> - Reduce includes
> - Merge remote-tracking branch 'upstream/master' into jdk-windows
> - os_windows: remove duplicated UMA handling
> - test_safefetch{32,N} works fine on win+aarch64
> - cleanup for 8253539: Remove unused JavaThread functions for
> set_last_Java_fp/pc
> - cleanup for 8253457: Remove unimplemented register stack functions
> - Merge remote-tracking branch 'upstream/master' into jdk-windows
> - ... and 14 more:
> https://git.openjdk.java.net/jdk/compare/ec9bee68...a7cdaad6
src/hotspot/cpu/aarch64/register_aarch64.cpp line 44:
> 42: "rscratch1", "rscratch2",
> 43: "r10", "r11", "r12", "r13", "r14", "r15", "r16",
> 44: "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls"), "r19",
For me this line doesn't look good in case of expanding this functionality to
macos-aarch64
as it's just the name of register and it's r18 on every platform except WIN64
and has nothing to do with reserving r18.
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
> This is a continuation of
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>
> Changes since then:
> * We've improved the write barrier as suggested by Andrew [1]
> * The define-guards around R18 have been changed to `R18_RESERVED`. This will
> be enabled for Windows only for now but
> will be required for the upcoming macOS+Aarch64 [2] port as well.
> * We've incorporated https://github.com/openjdk/jdk/pull/154 by @AntonKozlov
> in our PR for now and built the
> Windows-specific CPU feature detection on top of it.
>
> [1]
> https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009597.html
> [2] https://openjdk.java.net/jeps/8251280
Monica Beckwith has updated the pull request with a new target base due to a
merge or a rebase. The pull request now
contains 24 commits:
- Merge remote-tracking branch 'upstream/master' into jdk-windows
- SA: update copyright
- Fix graal codestyle
- Reduce includes
- Merge remote-tracking branch 'upstream/master' into jdk-windows
- os_windows: remove duplicated UMA handling
- test_safefetch{32,N} works fine on win+aarch64
- cleanup for 8253539: Remove unused JavaThread functions for
set_last_Java_fp/pc
- cleanup for 8253457: Remove unimplemented register stack functions
- Merge remote-tracking branch 'upstream/master' into jdk-windows
- ... and 14 more: https://git.openjdk.java.net/jdk/compare/ec9bee68...a7cdaad6
-
Changes: https://git.openjdk.java.net/jdk/pull/212/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=212=09
Stats: 2566 lines in 62 files changed: 2208 ins; 126 del; 232 mod
Patch: https://git.openjdk.java.net/jdk/pull/212.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/212/head:pull/212
PR: https://git.openjdk.java.net/jdk/pull/212
