[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