Bob,
Thank you for doing this. In the meanwhile, some of my fixes were pushed
that invalidated your diff, for which I apologize. Here is an updated
version of your patch which applies cleanly:
http://cr.openjdk.java.net/~avoitylov/webrev.8209093.02/
-Aleksei
On 23/09/2018 18:45, Boris Ulasevich wrote:
Hi Bob,
I checked your changes with jtreg and jck. I see no new fails
introduced by this change.
regards,
Boris
On 19.09.2018 13:30, Boris Ulasevich wrote:
Hi Bob,
Thank you for this job!
I have started testing. Will come back with results next week :)
The changes is Ok for me. Please see some minor comments below.
regards,
Boris
On 17.09.2018 20:49, Aleksey Shipilev 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.
Yes, the note looks like AArch64 specific comment, but I think it is
valid for arm32 too. So the change is Ok for me.
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?
I believe both (L3188 and L3363) comments should mention SP[0] (not
R4) as an input parameter now.
- // 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