# HG changeset patch # User never # Date 1570664144 25200 # Node ID f9cc0141574c9d31eb54fd456d2cb708a09c74c7 # Parent e84d8379815ba0d3e50fb096d28c25894cb50b8c 8231586: enlarge encoding space for OopMapValue offsets Reviewed-by: dlong diff -r e84d8379815b -r f9cc0141574c src/hotspot/share/compiler/oopMap.cpp --- a/src/hotspot/share/compiler/oopMap.cpp Wed Oct 09 12:21:28 2019 -0700 +++ b/src/hotspot/share/compiler/oopMap.cpp Wed Oct 09 16:35:44 2019 -0700 @@ -48,29 +48,25 @@ // OopMapStream -OopMapStream::OopMapStream(OopMap* oop_map, int oop_types_mask) { +OopMapStream::OopMapStream(OopMap* oop_map) { _stream = new CompressedReadStream(oop_map->write_stream()->buffer()); - _mask = oop_types_mask; _size = oop_map->omv_count(); _position = 0; _valid_omv = false; } -OopMapStream::OopMapStream(const ImmutableOopMap* oop_map, int oop_types_mask) { +OopMapStream::OopMapStream(const ImmutableOopMap* oop_map) { _stream = new CompressedReadStream(oop_map->data_addr()); - _mask = oop_types_mask; _size = oop_map->count(); _position = 0; _valid_omv = false; } void OopMapStream::find_next() { - while(_position++ < _size) { + if (_position++ < _size) { _omv.read_from(_stream); - if(((int)_omv.type() & _mask) > 0) { - _valid_omv = true; - return; - } + _valid_omv = true; + return; } _valid_omv = false; } @@ -140,16 +136,7 @@ assert( _locs_used[reg->value()] == OopMapValue::unused_value, "cannot insert twice" ); debug_only( _locs_used[reg->value()] = x; ) - OopMapValue o(reg, x); - - if(x == OopMapValue::callee_saved_value) { - // This can never be a stack location, so we don't need to transform it. - assert(optional->is_reg(), "Trying to callee save a stack location"); - o.set_content_reg(optional); - } else if(x == OopMapValue::derived_oop_value) { - o.set_content_reg(optional); - } - + OopMapValue o(reg, x, optional); o.write_on(write_stream()); increment_count(); } @@ -160,11 +147,6 @@ } -void OopMap::set_value(VMReg reg) { - // At this time, we don't need value entries in our OopMap. -} - - void OopMap::set_narrowoop(VMReg reg) { set_xxx(reg, OopMapValue::narrowoop_value, VMRegImpl::Bad()); } @@ -328,7 +310,7 @@ // changed before derived pointer offset has been collected) OopMapValue omv; { - OopMapStream oms(map,OopMapValue::derived_oop_value); + OopMapStream oms(map); if (!oms.is_done()) { #ifndef TIERED COMPILER1_PRESENT(ShouldNotReachHere();) @@ -340,27 +322,28 @@ #endif // !TIERED do { omv = oms.current(); - oop* loc = fr->oopmapreg_to_location(omv.reg(),reg_map); - guarantee(loc != NULL, "missing saved register"); - oop *derived_loc = loc; - oop *base_loc = fr->oopmapreg_to_location(omv.content_reg(), reg_map); - // Ignore NULL oops and decoded NULL narrow oops which - // equal to CompressedOops::base() when a narrow oop - // implicit null check is used in compiled code. - // The narrow_oop_base could be NULL or be the address - // of the page below heap depending on compressed oops mode. - if (base_loc != NULL && *base_loc != NULL && !CompressedOops::is_base(*base_loc)) { - derived_oop_fn(base_loc, derived_loc); + if (omv.type() == OopMapValue::derived_oop_value) { + oop* loc = fr->oopmapreg_to_location(omv.reg(),reg_map); + guarantee(loc != NULL, "missing saved register"); + oop *derived_loc = loc; + oop *base_loc = fr->oopmapreg_to_location(omv.content_reg(), reg_map); + // Ignore NULL oops and decoded NULL narrow oops which + // equal to CompressedOops::base() when a narrow oop + // implicit null check is used in compiled code. + // The narrow_oop_base could be NULL or be the address + // of the page below heap depending on compressed oops mode. + if (base_loc != NULL && *base_loc != NULL && !CompressedOops::is_base(*base_loc)) { + derived_oop_fn(base_loc, derived_loc); + } } oms.next(); } while (!oms.is_done()); } } - // We want coop and oop oop_types - int mask = OopMapValue::oop_value | OopMapValue::narrowoop_value; { - for (OopMapStream oms(map,mask); !oms.is_done(); oms.next()) { + // We want coop and oop oop_types + for (OopMapStream oms(map); !oms.is_done(); oms.next()) { omv = oms.current(); oop* loc = fr->oopmapreg_to_location(omv.reg(),reg_map); // It should be an error if no location can be found for a @@ -436,12 +419,14 @@ assert(map != NULL, "no ptr map found"); DEBUG_ONLY(int nof_callee = 0;) - for (OopMapStream oms(map, OopMapValue::callee_saved_value); !oms.is_done(); oms.next()) { + for (OopMapStream oms(map); !oms.is_done(); oms.next()) { OopMapValue omv = oms.current(); - VMReg reg = omv.content_reg(); - oop* loc = fr->oopmapreg_to_location(omv.reg(), reg_map); - reg_map->set_location(reg, (address) loc); - DEBUG_ONLY(nof_callee++;) + if (omv.type() == OopMapValue::callee_saved_value) { + VMReg reg = omv.content_reg(); + oop* loc = fr->oopmapreg_to_location(omv.reg(), reg_map); + reg_map->set_location(reg, (address) loc); + DEBUG_ONLY(nof_callee++;) + } } // Check that runtime stubs save all callee-saved registers @@ -452,25 +437,6 @@ #endif // COMPILER2 } -//============================================================================= -// Non-Product code - -#ifndef PRODUCT - -bool ImmutableOopMap::has_derived_pointer() const { -#if !defined(TIERED) && !INCLUDE_JVMCI - COMPILER1_PRESENT(return false); -#endif // !TIERED -#if COMPILER2_OR_JVMCI - OopMapStream oms(this,OopMapValue::derived_oop_value); - return oms.is_done(); -#else - return false; -#endif // COMPILER2_OR_JVMCI -} - -#endif //PRODUCT - // Printing code is present in product build for -XX:+PrintAssembly. static diff -r e84d8379815b -r f9cc0141574c src/hotspot/share/compiler/oopMap.hpp --- a/src/hotspot/share/compiler/oopMap.hpp Wed Oct 09 12:21:28 2019 -0700 +++ b/src/hotspot/share/compiler/oopMap.hpp Wed Oct 09 16:35:44 2019 -0700 @@ -53,7 +53,7 @@ public: // Constants - enum { type_bits = 4, + enum { type_bits = 2, register_bits = BitsPerShort - type_bits }; enum { type_shift = 0, @@ -64,19 +64,41 @@ register_mask = right_n_bits(register_bits), register_mask_in_place = register_mask << register_shift }; - enum oop_types { // must fit in type_bits - unused_value =0, // powers of 2, for masking OopMapStream - oop_value = 1, - narrowoop_value = 2, - callee_saved_value = 4, - derived_oop_value= 8 }; + enum oop_types { + oop_value, + narrowoop_value, + callee_saved_value, + derived_oop_value, + unused_value = -1 // Only used as a sentinel value + }; // Constructors OopMapValue () { set_value(0); set_content_reg(VMRegImpl::Bad()); } - OopMapValue (VMReg reg, oop_types t) { set_reg_type(reg, t); set_content_reg(VMRegImpl::Bad()); } - OopMapValue (VMReg reg, oop_types t, VMReg reg2) { set_reg_type(reg, t); set_content_reg(reg2); } - OopMapValue (CompressedReadStream* stream) { read_from(stream); } + OopMapValue (VMReg reg, oop_types t, VMReg reg2) { + set_reg_type(reg, t); + set_content_reg(reg2); + } + + private: + void set_reg_type(VMReg p, oop_types t) { + set_value((p->value() << register_shift) | t); + assert(reg() == p, "sanity check" ); + assert(type() == t, "sanity check" ); + } + void set_content_reg(VMReg r) { + if (is_callee_saved()) { + // This can never be a stack location, so we don't need to transform it. + assert(r->is_reg(), "Trying to callee save a stack location"); + } else if (is_derived_oop()) { + assert (r->is_valid(), "must have a valid VMReg"); + } else { + assert (!r->is_valid(), "valid VMReg not allowed"); + } + _content_reg = r->value(); + } + + public: // Archiving void write_on(CompressedWriteStream* stream) { stream->write_int(value()); @@ -94,15 +116,10 @@ // Querying bool is_oop() { return mask_bits(value(), type_mask_in_place) == oop_value; } - bool is_narrowoop() { return mask_bits(value(), type_mask_in_place) == narrowoop_value; } + bool is_narrowoop() { return mask_bits(value(), type_mask_in_place) == narrowoop_value; } bool is_callee_saved() { return mask_bits(value(), type_mask_in_place) == callee_saved_value; } bool is_derived_oop() { return mask_bits(value(), type_mask_in_place) == derived_oop_value; } - void set_oop() { set_value((value() & register_mask_in_place) | oop_value); } - void set_narrowoop() { set_value((value() & register_mask_in_place) | narrowoop_value); } - void set_callee_saved() { set_value((value() & register_mask_in_place) | callee_saved_value); } - void set_derived_oop() { set_value((value() & register_mask_in_place) | derived_oop_value); } - VMReg reg() const { return VMRegImpl::as_VMReg(mask_bits(value(), register_mask_in_place) >> register_shift); } oop_types type() const { return (oop_types)mask_bits(value(), type_mask_in_place); } @@ -110,15 +127,7 @@ return (p->value() == (p->value() & register_mask)); } - void set_reg_type(VMReg p, oop_types t) { - set_value((p->value() << register_shift) | t); - assert(reg() == p, "sanity check" ); - assert(type() == t, "sanity check" ); - } - - VMReg content_reg() const { return VMRegImpl::as_VMReg(_content_reg, true); } - void set_content_reg(VMReg r) { _content_reg = r->value(); } // Physical location queries bool is_register_loc() { return reg()->is_reg(); } @@ -156,6 +165,8 @@ enum DeepCopyToken { _deep_copy_token }; OopMap(DeepCopyToken, OopMap* source); // used only by deep_copy + void set_xxx(VMReg reg, OopMapValue::oop_types x, VMReg optional); + public: OopMap(int frame_size, int arg_count); @@ -173,19 +184,14 @@ // frame_size units are stack-slots (4 bytes) NOT intptr_t; we can name odd // slots to hold 4-byte values like ints and floats in the LP64 build. void set_oop ( VMReg local); - void set_value( VMReg local); void set_narrowoop(VMReg local); - void set_dead ( VMReg local); void set_callee_saved( VMReg local, VMReg caller_machine_register ); void set_derived_oop ( VMReg local, VMReg derived_from_local_register ); - void set_xxx(VMReg reg, OopMapValue::oop_types x, VMReg optional); int heap_size() const; void copy_data_to(address addr) const; OopMap* deep_copy(); - bool has_derived_pointer() const PRODUCT_RETURN0; - bool legal_vm_reg_name(VMReg local) { return OopMapValue::legal_vm_reg_name(local); } @@ -269,7 +275,6 @@ public: ImmutableOopMap(const OopMap* oopmap); - bool has_derived_pointer() const PRODUCT_RETURN0; int count() const { return _count; } #ifdef ASSERT int nr_of_bytes() const; // this is an expensive operation, only used in debug builds @@ -334,7 +339,6 @@ class OopMapStream : public StackObj { private: CompressedReadStream* _stream; - int _mask; int _size; int _position; bool _valid_omv; @@ -342,8 +346,8 @@ void find_next(); public: - OopMapStream(OopMap* oop_map, int oop_types_mask = OopMapValue::type_mask_in_place); - OopMapStream(const ImmutableOopMap* oop_map, int oop_types_mask = OopMapValue::type_mask_in_place); + OopMapStream(OopMap* oop_map); + OopMapStream(const ImmutableOopMap* oop_map); bool is_done() { if(!_valid_omv) { find_next(); } return !_valid_omv; } void next() { find_next(); } OopMapValue current() { return _omv; } diff -r e84d8379815b -r f9cc0141574c src/hotspot/share/opto/buildOopMap.cpp --- a/src/hotspot/share/opto/buildOopMap.cpp Wed Oct 09 12:21:28 2019 -0700 +++ b/src/hotspot/share/opto/buildOopMap.cpp Wed Oct 09 16:35:44 2019 -0700 @@ -352,7 +352,6 @@ } else { // Other - some reaching non-oop value - omap->set_value( r); #ifdef ASSERT if( t->isa_rawptr() && C->cfg()->_raw_oops.member(def) ) { def->dump(); @@ -377,11 +376,18 @@ #endif #ifdef ASSERT - for( OopMapStream oms1(omap, OopMapValue::derived_oop_value); !oms1.is_done(); oms1.next()) { + for( OopMapStream oms1(omap); !oms1.is_done(); oms1.next()) { OopMapValue omv1 = oms1.current(); + if (omv1.type() != OopMapValue::derived_oop_value) { + continue; + } bool found = false; - for( OopMapStream oms2(omap,OopMapValue::oop_value); !oms2.is_done(); oms2.next()) { - if( omv1.content_reg() == oms2.current().reg() ) { + for( OopMapStream oms2(omap); !oms2.is_done(); oms2.next()) { + OopMapValue omv2 = oms2.current(); + if (omv2.type() != OopMapValue::oop_value) { + continue; + } + if( omv1.content_reg() == omv2.reg() ) { found = true; break; } diff -r e84d8379815b -r f9cc0141574c src/hotspot/share/runtime/interfaceSupport.cpp --- a/src/hotspot/share/runtime/interfaceSupport.cpp Wed Oct 09 12:21:28 2019 -0700 +++ b/src/hotspot/share/runtime/interfaceSupport.cpp Wed Oct 09 16:35:44 2019 -0700 @@ -55,12 +55,6 @@ if (WalkStackALot) { InterfaceSupport::walk_stack(); } -#ifdef COMPILER2 - // This option is not used by Compiler 1 - if (StressDerivedPointers) { - InterfaceSupport::stress_derived_pointers(); - } -#endif if (DeoptimizeALot || DeoptimizeRandom) { InterfaceSupport::deoptimizeAll(); } @@ -234,31 +228,6 @@ } -void InterfaceSupport::stress_derived_pointers() { -#ifdef COMPILER2 - JavaThread *thread = JavaThread::current(); - if (!is_init_completed()) return; - ResourceMark rm(thread); - bool found = false; - for (StackFrameStream sfs(thread); !sfs.is_done() && !found; sfs.next()) { - CodeBlob* cb = sfs.current()->cb(); - if (cb != NULL && cb->oop_maps() ) { - // Find oopmap for current method - const ImmutableOopMap* map = cb->oop_map_for_return_address(sfs.current()->pc()); - assert(map != NULL, "no oopmap found for pc"); - found = map->has_derived_pointer(); - } - } - if (found) { - // $$$ Not sure what to do here. - /* - Scavenge::invoke(0); - */ - } -#endif -} - - void InterfaceSupport::verify_stack() { JavaThread* thread = JavaThread::current(); ResourceMark rm(thread); diff -r e84d8379815b -r f9cc0141574c src/hotspot/share/runtime/interfaceSupport.inline.hpp --- a/src/hotspot/share/runtime/interfaceSupport.inline.hpp Wed Oct 09 12:21:28 2019 -0700 +++ b/src/hotspot/share/runtime/interfaceSupport.inline.hpp Wed Oct 09 16:35:44 2019 -0700 @@ -62,7 +62,6 @@ static void zombieAll(); static void deoptimizeAll(); - static void stress_derived_pointers(); static void verify_stack(); static void verify_last_frame(); # endif diff -r e84d8379815b -r f9cc0141574c src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/compiler/OopMapValue.java --- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/compiler/OopMapValue.java Wed Oct 09 12:21:28 2019 -0700 +++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/compiler/OopMapValue.java Wed Oct 09 16:35:44 2019 -0700 @@ -47,7 +47,6 @@ static int REGISTER_MASK_IN_PLACE; // Types of OopValues - static int UNUSED_VALUE; static int OOP_VALUE; static int NARROWOOP_VALUE; static int CALLEE_SAVED_VALUE; @@ -70,7 +69,6 @@ TYPE_MASK_IN_PLACE = db.lookupIntConstant("OopMapValue::type_mask_in_place").intValue(); REGISTER_MASK = db.lookupIntConstant("OopMapValue::register_mask").intValue(); REGISTER_MASK_IN_PLACE = db.lookupIntConstant("OopMapValue::register_mask_in_place").intValue(); - UNUSED_VALUE = db.lookupIntConstant("OopMapValue::unused_value").intValue(); OOP_VALUE = db.lookupIntConstant("OopMapValue::oop_value").intValue(); NARROWOOP_VALUE = db.lookupIntConstant("OopMapValue::narrowoop_value").intValue(); CALLEE_SAVED_VALUE = db.lookupIntConstant("OopMapValue::callee_saved_value").intValue(); @@ -78,7 +76,6 @@ } public static abstract class OopTypes { - public static final OopTypes UNUSED_VALUE = new OopTypes() { int getValue() { return OopMapValue.UNUSED_VALUE; }}; public static final OopTypes OOP_VALUE = new OopTypes() { int getValue() { return OopMapValue.OOP_VALUE; }}; public static final OopTypes NARROWOOP_VALUE = new OopTypes() { int getValue() { return OopMapValue.NARROWOOP_VALUE; }}; public static final OopTypes CALLEE_SAVED_VALUE = new OopTypes() { int getValue() { return OopMapValue.CALLEE_SAVED_VALUE; }}; @@ -111,8 +108,7 @@ public OopTypes getType() { int which = (getValue() & TYPE_MASK_IN_PLACE); - if (which == UNUSED_VALUE) return OopTypes.UNUSED_VALUE; - else if (which == OOP_VALUE) return OopTypes.OOP_VALUE; + if (which == OOP_VALUE) return OopTypes.OOP_VALUE; else if (which == NARROWOOP_VALUE) return OopTypes.NARROWOOP_VALUE; else if (which == CALLEE_SAVED_VALUE) return OopTypes.CALLEE_SAVED_VALUE; else if (which == DERIVED_OOP_VALUE) return OopTypes.DERIVED_OOP_VALUE;