[8u] RFR: JDK-8257192: Integrate AArch64 JIT port into 8u
Aleksey Shipilev
shade at redhat.com
Wed Jan 27 16:15:30 UTC 2021
On 1/25/21 8:43 PM, Andrew Hughes wrote:
>>>> *) src/share/vm/c1/c1_Runtime1.hpp
>>>> - so, "move_klass_patching" is undefined for AARCH64 (see .cpp change), should it be undeclared too?
>>>
>>> It's in src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp
>>
>> Yes, but under TARGET_ARCH_aarch64, *_patching and friends remains _only_
>> declared, not defined. Shouldn't the entire *_patching declaration block be
>> e.g. this?
>>
>> #ifdef TARGET_ARCH_aarch64
>> static void patch_code_aarch64(JavaThread* thread, StubID stub_id);
>> #else
>> static int access_field_patching(JavaThread* thread);
>> static int move_klass_patching(JavaThread* thread);
>> static int move_mirror_patching(JavaThread* thread);
>> static int move_appendix_patching(JavaThread* thread);
>> static void patch_code(JavaThread* thread, StubID stub_id);
>> #endif
>
> Still not sure what you're aiming at here. The routines are defined in
> c1_Runtime1_aarch64.cpp. If they were not defined anywhere, surely
> the build would fail? Given how well tested the current version is,
> I'd prefer to leave it as is in the current patch, so as to not to
> deviate too far in the initial import or delay this any
> further. You're welcome to change this in a follow-up patch.
Yeah, it is not a build breakage to define the method and not implement it, as long as nobody tries
to reference the method. When that happens, linker would complain. But I agree about leaving it as
is for integration sanity.
> I think breakage is unlikely - and that's why I'm trying to minimise differences
> from what's already in aarch64/shenandoah-jdk8u where possible - but we're now
> aiming for 8u292.
>
> https://cr.openjdk.java.net/~andrew/openjdk8/8257192/hotspot/webrev.03/
>
> has the first change listed above and also JDK-8221725, following the inclusion
> of JDK-8221408 in 8u292.
>
> Differences look like this:
>
> $ diff -u ../../webrevs/openjdk8/8257192/hotspot/webrev.02/hotspot.patch ../../webrevs/openjdk8/8257192/hotspot/webrev.03/hotspot.aarch64.patch |egrep '^[+-][+-] '
> -+ _matrule->equivalent(AD.globalNames(), short_branch->_matrule) &&
> -+ AARCH64_ONLY(equivalent_predicates(this, short_branch)) NOT_AARCH64(true)) {
> ++ _matrule->equivalent(AD.globalNames(), short_branch->_matrule)
> ++ AARCH64_ONLY(&& equivalent_predicates(this, short_branch))) {
> -+ __ mov(tmp, (address) (~(os::vm_page_size()-1) | markOopDesc::lock_mask_in_place));
> ++ __ mov(tmp, (address) (~(os::vm_page_size()-1) | (uintptr_t)markOopDesc::lock_mask_in_place));
>
> Doing a test build with this now across all architectures. Let's finally get this in.
Okay, this looks good to me.
We are (hopefully) done!
--
Thanks,
-Aleksey
More information about the jdk8u-dev
mailing list