[aarch64-port-dev ] RFR: Fix for UseCompressedClassPointers
Edward Nevill
edward.nevill at linaro.org
Wed Oct 16 04:17:45 PDT 2013
Hi,
The following patch fixes UseCompressedClassPointers for C1 and C2 and re-enables them by default for C2.
Basically the change is that there are now 2 narrow bases, narrow_klass_base and narrow_oop_base. narrow_oop_base is kept in rheapbase as before.
This means that in encode_klass and decode_klass we must add/sub narrow_klass_base and we do not have it conveniently in a register.
Unfortunately rscratch1 and rscratch2 are variously used by callers of encode_klass and decode_klass. So what I do is use rheapbase as a scratch register if necessary, and then call reinit_heapbase afterwards to restore it. This mirrors what is done on x86.
It is not possible to call add(...) to simply add/sub either the narrow_klass_base or the delta between the narrow_klass_base and the narrow_oop_base because of the following in MacroAssembler::wrap_add_sub_imm_insn(...)
...
assert_different_registers(Rd, Rn);
mov(Rd, (uint64_t)imm);
(this->*insn2)(Rd, Rn, Rd, LSL, 0);
...
Because of the limited range of add/sub handled on aarch64 (+/- 1<<24) we cannot use add/sub unless we can guarantee that the src and dst registers are different which leads to us needing a scratch register so we are back where we started.
I have also made a change in aarch64.ad to fix a case where encode_heap_oop_not_null was in fact being called with a null constant. Also I remove some bogus guarantees from nativeInst_aarch64 to make it work for C2.
I know the above solution is not ideal and we would prefer a more optimal solution, however it is at least correct so I would like to push it and look at ways of optimising later.
Possible optimisation is to add an additional arg to encode/decode klass which is the scratch register to be used. Then the callers can either pass in rscratch1, rscratch2, or rheapbase if they genuinely do not have a free scratch register.
Ok to push?
Ed.
--- CUT HERE ---
exporting patch:
# HG changeset patch
# User Edward Nevill edward.nevill at linaro.org
# Date 1381920878 -3600
# Wed Oct 16 11:54:38 2013 +0100
# Node ID 778fdde3772e321c55fefa3df6f22c3e7d49cd33
# Parent a99f56e36ea4ad581c901c44a27906919485e30c
Fix UseCompressedClassPointers
diff -r a99f56e36ea4 -r 778fdde3772e src/cpu/aarch64/vm/aarch64.ad
--- a/src/cpu/aarch64/vm/aarch64.ad Mon Oct 14 09:52:17 2013 +0100
+++ b/src/cpu/aarch64/vm/aarch64.ad Wed Oct 16 11:54:38 2013 +0100
@@ -2375,7 +2375,7 @@
// need to do this the hard way until we can manage relocs
// for 32 bit constants
__ movoop(rscratch2, (jobject)con);
- __ encode_heap_oop_not_null(rscratch2);
+ if (con) __ encode_heap_oop_not_null(rscratch2);
if (index == -1) {
__ strw(rscratch2, Address(base, disp));
} else {
@@ -2621,7 +2621,7 @@
// need to do this the hard way until we can manage relocs
// for 32 bit constants
__ movoop(rscratch2, (jobject)con);
- __ encode_heap_oop_not_null(rscratch2);
+ if (con) __ encode_heap_oop_not_null(rscratch2);
}
MOV_VOLATILE(rscratch2, $mem$$base, $mem$$index, $mem$$scale, $mem$$disp,
rscratch1, stlrw);
@@ -2634,8 +2634,9 @@
// need to do this the hard way until we can manage relocs
// for 32 bit constants
__ movoop(rscratch2, (jobject)con);
- __ encode_heap_oop_not_null(rscratch2);
- __ encode_klass_not_null(rscratch2);
+ // Either it is a heap oop or a klass pointer? It can't be both?
+ // __ encode_heap_oop_not_null(rscratch2);
+ if (con) __ encode_klass_not_null(rscratch2);
}
MOV_VOLATILE(rscratch2, $mem$$base, $mem$$index, $mem$$scale, $mem$$disp,
rscratch1, stlrw);
diff -r a99f56e36ea4 -r 778fdde3772e src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp Mon Oct 14 09:52:17 2013 +0100
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp Wed Oct 16 11:54:38 2013 +0100
@@ -1644,8 +1644,8 @@
void MacroAssembler::reinit_heapbase()
{
if (UseCompressedOops) {
- lea(rscratch1, ExternalAddress((address)Universe::narrow_ptrs_base_addr()));
- ldr(rheapbase, Address(rscratch1));
+ lea(rheapbase, ExternalAddress((address)Universe::narrow_ptrs_base_addr()));
+ ldr(rheapbase, Address(rheapbase));
}
}
@@ -2078,73 +2078,46 @@
}
}
-void MacroAssembler::encode_klass_not_null(Register r) {
-#ifdef ASSERT
- verify_heapbase("MacroAssembler::encode_klass_not_null: heap base corrupted?");
-#endif
- if (Universe::narrow_klass_base() != NULL) {
- sub(r, r, rheapbase);
- }
- if (Universe::narrow_klass_shift() != 0) {
- assert (LogKlassAlignmentInBytes == Universe::narrow_klass_shift(), "decode alg wrong");
- lsr(r, r, LogKlassAlignmentInBytes);
- }
-}
-
-void MacroAssembler::encode_klass_not_null(Register dst, Register src) {
-#ifdef ASSERT
- verify_heapbase("MacroAssembler::encode_klass_not_null2: heap base corrupted?");
-#endif
- if (dst != src) {
- mov(dst, src);
- }
- if (Universe::narrow_klass_base() != NULL) {
- sub(dst, dst, rheapbase);
- }
- if (Universe::narrow_klass_shift() != 0) {
- assert (LogKlassAlignmentInBytes == Universe::narrow_klass_shift(), "decode alg wrong");
- lsr(dst, dst, LogKlassAlignmentInBytes);
- }
-}
-
-void MacroAssembler::decode_klass_not_null(Register r) {
- // Note: it will change flags
- assert (UseCompressedClassPointers, "should only be used for compressed headers");
- // Cannot assert, unverified entry point counts instructions (see .ad file)
- // vtableStubs also counts instructions in pd_code_size_limit.
- // Also do not verify_oop as this is called by verify_oop.
- if (Universe::narrow_klass_shift() != 0) {
- assert(LogKlassAlignmentInBytes == Universe::narrow_klass_shift(), "decode alg wrong");
- if (Universe::narrow_klass_base() != NULL) {
- add(r, rheapbase, r, Assembler::LSL, LogKlassAlignmentInBytes);
- } else {
- add(r, zr, r, Assembler::LSL, LogKlassAlignmentInBytes);
- }
- } else {
- assert (Universe::narrow_klass_base() == NULL, "sanity");
- }
-}
-
-void MacroAssembler::decode_klass_not_null(Register dst, Register src) {
- // Note: it will change flags
- assert (UseCompressedClassPointers, "should only be used for compressed headers");
- // Cannot assert, unverified entry point counts instructions (see .ad file)
- // vtableStubs also counts instructions in pd_code_size_limit.
- // Also do not verify_oop as this is called by verify_oop.
- if (Universe::narrow_klass_shift() != 0) {
- assert(LogKlassAlignmentInBytes == Universe::narrow_klass_shift(), "decode alg wrong");
- if (Universe::narrow_klass_base() != NULL) {
- add(dst, rheapbase, src, Assembler::LSL, LogKlassAlignmentInBytes);
- } else {
- add(dst, zr, src, Assembler::LSL, LogKlassAlignmentInBytes);
- }
- } else {
- assert (Universe::narrow_klass_base() == NULL, "sanity");
- if (dst != src) {
- mov(dst, src);
- }
- }
-}
+void MacroAssembler::encode_klass_not_null(Register dst, Register src) {
+ Register rbase = dst;
+#ifdef ASSERT
+ verify_heapbase("MacroAssembler::encode_klass_not_null2: heap base corrupted?");
+#endif
+ if (dst == src) rbase = rheapbase;
+ mov(rbase, (uint64_t)Universe::narrow_klass_base());
+ sub(dst, src, rbase);
+ if (Universe::narrow_klass_shift() != 0) {
+ assert (LogKlassAlignmentInBytes == Universe::narrow_klass_shift(), "decode alg wrong");
+ lsr(dst, dst, LogKlassAlignmentInBytes);
+ }
+ if (dst == src) reinit_heapbase();
+}
+
+void MacroAssembler::encode_klass_not_null(Register r) {
+ encode_klass_not_null(r, r);
+}
+
+void MacroAssembler::decode_klass_not_null(Register dst, Register src) {
+ Register rbase = dst;
+ assert(Universe::narrow_klass_base() != NULL, "Base should be initialized");
+ assert (UseCompressedClassPointers, "should only be used for compressed headers");
+ // Cannot assert, unverified entry point counts instructions (see .ad file)
+ // vtableStubs also counts instructions in pd_code_size_limit.
+ // Also do not verify_oop as this is called by verify_oop.
+ if (dst == src) rbase = rheapbase;
+ mov(rbase, (uint64_t)Universe::narrow_klass_base());
+ if (Universe::narrow_klass_shift() != 0) {
+ assert(LogKlassAlignmentInBytes == Universe::narrow_klass_shift(), "decode alg wrong");
+ add(dst, rbase, src, Assembler::LSL, LogKlassAlignmentInBytes);
+ } else {
+ add(dst, rbase, src);
+ }
+ if (dst == src) reinit_heapbase();
+}
+
+void MacroAssembler::decode_klass_not_null(Register r) {
+ decode_klass_not_null(r, r);
+}
// TODO
//
diff -r a99f56e36ea4 -r 778fdde3772e src/cpu/aarch64/vm/nativeInst_aarch64.cpp
--- a/src/cpu/aarch64/vm/nativeInst_aarch64.cpp Mon Oct 14 09:52:17 2013 +0100
+++ b/src/cpu/aarch64/vm/nativeInst_aarch64.cpp Wed Oct 16 11:54:38 2013 +0100
@@ -143,19 +143,6 @@
void NativeJump::insert(address code_pos, address entry) { Unimplemented(); }
void NativeJump::check_verified_entry_alignment(address entry, address verified_entry) {
- // Patching to not_entrant can happen while activations of the method are
- // in use. The patching in that instance must happen only when certain
- // alignment restrictions are true. These guarantees check those
- // conditions.
- const int linesize = 64;
-
- // Must be wordSize aligned
- guarantee(((uintptr_t) verified_entry & (wordSize -1)) == 0,
- "illegal address for code patching 2");
- // First 5 bytes must be within the same cache line - 4827828
- guarantee((uintptr_t) verified_entry / linesize ==
- ((uintptr_t) verified_entry + 4) / linesize,
- "illegal address for code patching 3");
}
diff -r a99f56e36ea4 -r 778fdde3772e src/share/vm/runtime/arguments.cpp
--- a/src/share/vm/runtime/arguments.cpp Mon Oct 14 09:52:17 2013 +0100
+++ b/src/share/vm/runtime/arguments.cpp Wed Oct 16 11:54:38 2013 +0100
@@ -1471,13 +1471,10 @@
}
FLAG_SET_DEFAULT(UseCompressedClassPointers, false);
} else {
-// ECN: FIXME - UseCompressedClassPointers is temporarily broken
-#ifndef AARCH64
// Turn on UseCompressedClassPointers too
if (FLAG_IS_DEFAULT(UseCompressedClassPointers)) {
FLAG_SET_ERGO(bool, UseCompressedClassPointers, true);
}
-#endif
// Check the CompressedClassSpaceSize to make sure we use compressed klass ptrs.
if (UseCompressedClassPointers) {
if (CompressedClassSpaceSize > KlassEncodingMetaspaceMax) {
--- CUT HERE ---
More information about the aarch64-port-dev
mailing list