[aarch64-port-dev ] [8u] RFR: JDK-8257192: Integrate AArch64 JIT port into 8u

Aleksey Shipilev shade at redhat.com
Mon Nov 30 13:45:38 UTC 2020


Hi,

Thanks for explanations. These are the only things left from my review:

On 11/27/20 8:23 PM, Andrew Hughes wrote:
>> *) src/share/vm/adlc/formssel.cpp:
>>    - equivalent_predicates addition changes the shared code path. Is this a bugfix?
>>
> 
> 8145438: Guarantee failures since 8144028: Use AArch64 bit-test instructions in C2
> 
> Made AARCH64_ONLY.

You can write this:

1246       _matrule->equivalent(AD.globalNames(), short_branch->_matrule) &&
1247       AARCH64_ONLY(equivalent_predicates(this, short_branch)) NOT_AARCH64(true)) {

as:

1246       _matrule->equivalent(AD.globalNames(), short_branch->_matrule)
1247       AARCH64_ONLY(&& equivalent_predicates(this, short_branch))) {


>> *) 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

> https://cr.openjdk.java.net/~andrew/openjdk8/8257192/hotspot/webrev.02/

I have only a philosophical point left: if we push this to 8u282, this means we start the clock on 
getting any remaining issues resolved in December before the January release. Since December is the 
time when a significant part of involved people take vacations, I think we are risking this quite a 
bit. While not exactly the concern of OpenJDK 8u Upstream, it would also mean we would need to merge 
this back to aarch64-port/jdk8u-shenandoah, and make sure that Shenandoah itself is not broken.

It would make me sleep and relax better if we integrate it first thing in 8u292 (for April CPU), so 
that we would have plenty of time next year to work out the kinks, if any. This is ultimately 
something for 8u Maintainers to decide, of course.

-- 
Thanks,
-Aleksey



More information about the aarch64-port-dev mailing list