RFR: 8209093 - JEP 340: One AArch64 Port, Not Two

[Bob Vandette]

Thanks for the detailed review Aleksey. I took care of these.

Bob.

> On Sep 17, 2018, at 1:49 PM, Aleksey Shipilev <shade at redhat.com> wrote:
> 
> On 09/17/2018 07:00 PM, Vladimir Kozlov wrote:
>> On 9/17/18 9:20 AM, Bob Vandette wrote:
>>> Please review the changes related to JEP 340 which remove the second
>>> 64-bit ARM port from the JDK.>>
>>> http://cr.openjdk.java.net/~bobv/8209093/webrev.01
> 
> I read through the webrev, and it seems to be the clean removal. It also feels
> very satisfying to drop 15 KLOC of ifdef-ed code.
> 
> Minor nits:
> 
> *) In src/hotspot/cpu/arm/c1_LIRAssembler_arm.cpp and
> src/hotspot/cpu/arm/interp_masm_arm.cpp we have "#if defined(ASSERT)", which cab
> be just "#ifdef ASSERT" now
> 
> *) Ditto in src/hotspot/cpu/arm/jniFastGetField_arm.cpp we have "#if
> defined(__ABI_HARD__)"
> 
> *) In src/hotspot/cpu/arm/macroAssembler_arm.hpp, the comment below seems to
> apply to AArch64 only. Yet, only the first line of the comment is removed. I
> think we should instead convert that comment into "TODO:" mentioning this might
> be revised after AArch64 removal?
> 
> 992   void branch_if_negative_32(Register r, Label& L) {
> 993     // Note about branch_if_negative_32() / branch_if_any_negative_32()
> implementation for AArch64:
> 994     // tbnz is not used instead of tst & b.mi because destination may be
> out of tbnz range (+-32KB)
> 995     // since these methods are used in LIR_Assembler::emit_arraycopy() to
> jump to stub entry.
> 996     tst_32(r, r);
> 997     b(L, mi);
> 998   }
> 
> *) In src/hotspot/cpu/arm/stubGenerator_arm.cpp, lines L1204..1205, L1435..1436
> can be merged together:
> 
> 1203   // Generate the inner loop for shifted forward array copy (unaligned copy).
> 1204   // It can be used when bytes_per_count < wordSize, i.e.
> 1205   //  byte/short copy
> 
> 1434   // Generate the inner loop for shifted backward array copy (unaligned copy).
> 1435   // It can be used when bytes_per_count < wordSize, i.e.
> 1436   //  byte/short copy
> 
> 
> *) In src/hotspot/cpu/arm/stubGenerator_arm.cpp, the comment changed
> incorrectly around L3363?
> 
>  - //    R4 (AArch64) / SP[0] (32-bit ARM) -  element count (32-bit int)
>  + //    R4    -  element count (32-bit int)
> 
> 
> *) In src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp, "R4"/"R5"
> comments are missing:
> 
>  - const Register Rsig_handler    = AARCH64_ONLY(R21) NOT_AARCH64(Rtmp_save0 /*
> R4 */);
>  - const Register Rnative_code    = AARCH64_ONLY(R22) NOT_AARCH64(Rtmp_save1 /*
> R5 */);
>  + const Register Rsig_handler    = Rtmp_save0;
>  + const Register Rnative_code    = Rtmp_save1;
> 
> Thanks,
> -Aleksey
>