8231586: enlarge encoding space for OopMapValue offsets
authornever
Wed, 09 Oct 2019 16:35:44 -0700
changeset 58527 f9cc0141574c
parent 58524 e84d8379815b
child 58528 d0519b8bd8d9
8231586: enlarge encoding space for OopMapValue offsets Reviewed-by: dlong
src/hotspot/share/compiler/oopMap.cpp
src/hotspot/share/compiler/oopMap.hpp
src/hotspot/share/opto/buildOopMap.cpp
src/hotspot/share/runtime/interfaceSupport.cpp
src/hotspot/share/runtime/interfaceSupport.inline.hpp
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/compiler/OopMapValue.java
--- 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
--- 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; }
--- 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;
       }
--- 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);
--- 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
--- 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;