Merge upstream-jdk

src/hotspot/share/c1/c1_RangeCheckElimination.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -483,14 +483,14 @@ void RangeCheckEliminator::in_block_motion(BlockBegin *block, AccessIndexedList
 
           if (c) {
             jint value = c->type()->as_IntConstant()->value();
-            if (value != min_jint) {
-              if (ao->op() == Bytecodes::_isub) {
-                value = -value;
-              }
+            if (ao->op() == Bytecodes::_iadd) {
               base = java_add(base, value);
-              last_integer = base;
-              last_instruction = other;
+            } else {
+              assert(ao->op() == Bytecodes::_isub, "unexpected bytecode");
+              base = java_subtract(base, value);
             }
+            last_integer = base;
+            last_instruction = other;
             index = other;
           } else {
             break;

src/hotspot/share/classfile/symbolTable.cpp

@@ -344,8 +344,23 @@ Symbol* SymbolTable::lookup_common(const char* name,
   return sym;
 }
 
+// Symbols should represent entities from the constant pool that are
+// limited to <64K in length, but usage errors creep in allowing Symbols
+// to be used for arbitrary strings. For debug builds we will assert if
+// a string is too long, whereas product builds will truncate it.
+static int check_length(const char* name, int len) {
+  assert(len <= Symbol::max_length(),
+         "String length %d exceeds the maximum Symbol length of %d", len, Symbol::max_length());
+  if (len > Symbol::max_length()) {
+    warning("A string \"%.80s ... %.80s\" exceeds the maximum Symbol "
+            "length of %d and has been truncated", name, (name + len - 80), Symbol::max_length());
+    len = Symbol::max_length();
+  }
+  return len;
+}
+
 Symbol* SymbolTable::new_symbol(const char* name, int len) {
-  assert(len <= Symbol::max_length(), "sanity");
+  len = check_length(name, len);
   unsigned int hash = hash_symbol(name, len, _alt_hash);
   Symbol* sym = lookup_common(name, len, hash);
   if (sym == nullptr) {
@@ -485,6 +500,7 @@ void SymbolTable::new_symbols(ClassLoaderData* loader_data, const constantPoolHa
   for (int i = 0; i < names_count; i++) {
     const char *name = names[i];
     int len = lengths[i];
+    assert(len <= Symbol::max_length(), "must be - these come from the constant pool");
     unsigned int hash = hashValues[i];
     assert(lookup_shared(name, len, hash) == nullptr, "must have checked already");
     Symbol* sym = do_add_if_needed(name, len, hash, is_permanent);
@@ -494,6 +510,7 @@ void SymbolTable::new_symbols(ClassLoaderData* loader_data, const constantPoolHa
 }
 
 Symbol* SymbolTable::do_add_if_needed(const char* name, int len, uintx hash, bool is_permanent) {
+  assert(len <= Symbol::max_length(), "caller should have ensured this");
   SymbolTableLookup lookup(name, len, hash);
   SymbolTableGet stg;
   bool clean_hint = false;
@@ -542,7 +559,7 @@ Symbol* SymbolTable::do_add_if_needed(const char* name, int len, uintx hash, boo
 
 Symbol* SymbolTable::new_permanent_symbol(const char* name) {
   unsigned int hash = 0;
-  int len = (int)strlen(name);
+  int len = check_length(name, (int)strlen(name));
   Symbol* sym = SymbolTable::lookup_only(name, len, hash);
   if (sym == nullptr) {
     sym = do_add_if_needed(name, len, hash, /* is_permanent */ true);

src/hotspot/share/interpreter/oopMapCache.cpp

@@ -66,9 +66,6 @@ class OopMapCacheEntry: private InterpreterOopMap {
  public:
   OopMapCacheEntry() : InterpreterOopMap() {
     _next = nullptr;
-#ifdef ASSERT
-    _resource_allocate_bit_mask = false;
-#endif
   }
 };
 
@@ -177,9 +174,13 @@ class VerifyClosure : public OffsetClosure {
 
 InterpreterOopMap::InterpreterOopMap() {
   initialize();
-#ifdef ASSERT
-  _resource_allocate_bit_mask = true;
-#endif
+}
+
+InterpreterOopMap::~InterpreterOopMap() {
+  if (has_valid_mask() && mask_size() > small_mask_limit) {
+    assert(_bit_mask[0] != 0, "should have pointer to C heap");
+    FREE_C_HEAP_ARRAY(uintptr_t, _bit_mask[0]);
+  }
 }
 
 bool InterpreterOopMap::is_empty() const {
@@ -399,37 +400,24 @@ void OopMapCacheEntry::deallocate(OopMapCacheEntry* const entry) {
 
 // Implementation of OopMapCache
 
-void InterpreterOopMap::resource_copy(OopMapCacheEntry* from) {
-  assert(_resource_allocate_bit_mask,
-    "Should not resource allocate the _bit_mask");
-  assert(from->has_valid_mask(),
-    "Cannot copy entry with an invalid mask");
+void InterpreterOopMap::copy_from(const OopMapCacheEntry* src) {
+  // The expectation is that this InterpreterOopMap is recently created
+  // and empty. It is used to get a copy of a cached entry.
+  assert(!has_valid_mask(), "InterpreterOopMap object can only be filled once");
+  assert(src->has_valid_mask(), "Cannot copy entry with an invalid mask");
 
-  set_method(from->method());
-  set_bci(from->bci());
-  set_mask_size(from->mask_size());
-  set_expression_stack_size(from->expression_stack_size());
-  _num_oops = from->num_oops();
+  set_method(src->method());
+  set_bci(src->bci());
+  set_mask_size(src->mask_size());
+  set_expression_stack_size(src->expression_stack_size());
+  _num_oops = src->num_oops();
 
   // Is the bit mask contained in the entry?
-  if (from->mask_size() <= small_mask_limit) {
-    memcpy((void *)_bit_mask, (void *)from->_bit_mask,
-      mask_word_size() * BytesPerWord);
+  if (src->mask_size() <= small_mask_limit) {
+    memcpy(_bit_mask, src->_bit_mask, mask_word_size() * BytesPerWord);
   } else {
-    // The expectation is that this InterpreterOopMap is a recently created
-    // and empty. It is used to get a copy of a cached entry.
-    // If the bit mask has a value, it should be in the
-    // resource area.
-    assert(_bit_mask[0] == 0 ||
-      Thread::current()->resource_area()->contains((void*)_bit_mask[0]),
-      "The bit mask should have been allocated from a resource area");
-    // Allocate the bit_mask from a Resource area for performance.  Allocating
-    // from the C heap as is done for OopMapCache has a significant
-    // performance impact.
-    _bit_mask[0] = (uintptr_t) NEW_RESOURCE_ARRAY(uintptr_t, mask_word_size());
-    assert(_bit_mask[0] != 0, "bit mask was not allocated");
-    memcpy((void*) _bit_mask[0], (void*) from->_bit_mask[0],
-      mask_word_size() * BytesPerWord);
+    _bit_mask[0] = (uintptr_t) NEW_C_HEAP_ARRAY(uintptr_t, mask_word_size(), mtClass);
+    memcpy((void*) _bit_mask[0], (void*) src->_bit_mask[0], mask_word_size() * BytesPerWord);
   }
 }
 
@@ -516,7 +504,7 @@ void OopMapCache::lookup(const methodHandle& method,
     for (int i = 0; i < _probe_depth; i++) {
       OopMapCacheEntry *entry = entry_at(probe + i);
       if (entry != nullptr && !entry->is_empty() && entry->match(method, bci)) {
-        entry_for->resource_copy(entry);
+        entry_for->copy_from(entry);
         assert(!entry_for->is_empty(), "A non-empty oop map should be returned");
         log_debug(interpreter, oopmap)("- found at hash %d", probe + i);
         return;
@@ -530,7 +518,7 @@ void OopMapCache::lookup(const methodHandle& method,
   OopMapCacheEntry* tmp = NEW_C_HEAP_OBJ(OopMapCacheEntry, mtClass);
   tmp->initialize();
   tmp->fill(method, bci);
-  entry_for->resource_copy(tmp);
+  entry_for->copy_from(tmp);
 
   if (method->should_not_be_cached()) {
     // It is either not safe or not a good idea to cache this Method*
@@ -631,7 +619,7 @@ void OopMapCache::compute_one_oop_map(const methodHandle& method, int bci, Inter
   tmp->initialize();
   tmp->fill(method, bci);
   if (tmp->has_valid_mask()) {
-    entry->resource_copy(tmp);
+    entry->copy_from(tmp);
   }
   OopMapCacheEntry::deallocate(tmp);
 }

src/hotspot/share/interpreter/oopMapCache.hpp

@@ -36,13 +36,14 @@
 // OopMapCache's are allocated lazily per InstanceKlass.
 
 // The oopMap (InterpreterOopMap) is stored as a bit mask. If the
-// bit_mask can fit into two words it is stored in
+// bit_mask can fit into four words it is stored in
 // the _bit_mask array, otherwise it is allocated on the heap.
 // For OopMapCacheEntry the bit_mask is allocated in the C heap
 // because these entries persist between garbage collections.
-// For InterpreterOopMap the bit_mask is allocated in
-// a resource area for better performance.  InterpreterOopMap
-// should only be created and deleted during same garbage collection.
+// For InterpreterOopMap the bit_mask is allocated in the C heap
+// to avoid issues with allocations from the resource area that have
+// to live accross the oop closure. InterpreterOopMap should only be
+// created and deleted during the same garbage collection.
 //
 // If ENABBLE_ZAP_DEAD_LOCALS is defined, two bits are used
 // per entry instead of one. In all cases,
@@ -95,9 +96,6 @@ class InterpreterOopMap: ResourceObj {
                                   // access it without using trickery in
                                   // method bit_mask().
   int            _num_oops;
-#ifdef ASSERT
-  bool _resource_allocate_bit_mask;
-#endif
 
   // access methods
   Method*        method() const                  { return _method; }
@@ -128,12 +126,13 @@ class InterpreterOopMap: ResourceObj {
 
  public:
   InterpreterOopMap();
+  ~InterpreterOopMap();
 
-  // Copy the OopMapCacheEntry in parameter "from" into this
-  // InterpreterOopMap.  If the _bit_mask[0] in "from" points to
-  // allocated space (i.e., the bit mask was to large to hold
-  // in-line), allocate the space from a Resource area.
-  void resource_copy(OopMapCacheEntry* from);
+  // Copy the OopMapCacheEntry in parameter "src" into this
+  // InterpreterOopMap.  If the _bit_mask[0] in "src" points to
+  // allocated space (i.e., the bit mask was too large to hold
+  // in-line), allocate the space from the C heap.
+  void copy_from(const OopMapCacheEntry* src);
 
   void iterate_oop(OffsetClosure* oop_closure) const;
   void print() const;

src/hotspot/share/oops/symbol.cpp

@@ -54,6 +54,7 @@ uint32_t Symbol::pack_hash_and_refcount(short hash, int refcount) {
 }
 
 Symbol::Symbol(const u1* name, int length, int refcount) {
+  assert(length <= max_length(), "SymbolTable should have caught this!");
   _hash_and_refcount =  pack_hash_and_refcount((short)os::random(), refcount);
   _length = (u2)length;
   // _body[0..1] are allocated in the header just by coincidence in the current

src/hotspot/share/oops/symbol.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -130,6 +130,7 @@ class Symbol : public MetaspaceObj {
     return (int)heap_word_size(byte_size(length));
   }
 
+  // Constructor is private for use only by SymbolTable.
   Symbol(const u1* name, int length, int refcount);
 
   static short extract_hash(uint32_t value)   { return (short)(value >> 16); }

src/hotspot/share/opto/ifnode.cpp

@@ -755,6 +755,7 @@ bool IfNode::cmpi_folds(PhaseIterGVN* igvn, bool fold_ne) {
 bool IfNode::is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn) {
   return ctrl != nullptr &&
     ctrl->is_Proj() &&
+    ctrl->outcnt() == 1 && // No side-effects
     ctrl->in(0) != nullptr &&
     ctrl->in(0)->Opcode() == Op_If &&
     ctrl->in(0)->outcnt() == 2 &&
@@ -1328,7 +1329,7 @@ Node* IfNode::fold_compares(PhaseIterGVN* igvn) {
 
   if (cmpi_folds(igvn)) {
     Node* ctrl = in(0);
-    if (is_ctrl_folds(ctrl, igvn) && ctrl->outcnt() == 1) {
+    if (is_ctrl_folds(ctrl, igvn)) {
       // A integer comparison immediately dominated by another integer
       // comparison
       ProjNode* success = nullptr;

src/hotspot/share/opto/machnode.hpp

@@ -870,6 +870,10 @@ class MachSafePointNode : public MachReturnNode {
     assert(verify_jvms(jvms), "jvms must match");
     return in(_jvmadj + jvms->monitor_box_offset(idx));
   }
+  Node* scalarized_obj(const JVMState* jvms, uint idx) const {
+    assert(verify_jvms(jvms), "jvms must match");
+    return in(_jvmadj + jvms->scloff() + idx);
+  }
   void  set_local(const JVMState* jvms, uint idx, Node *c) {
     assert(verify_jvms(jvms), "jvms must match");
     set_req(_jvmadj + jvms->locoff() + idx, c);

src/hotspot/share/opto/output.cpp

@@ -974,6 +974,27 @@ bool PhaseOutput::contains_as_owner(GrowableArray<MonitorValue*> *monarray, Obje
   return false;
 }
 
+// Determine if there is a scalar replaced object description represented by 'ov'.
+bool PhaseOutput::contains_as_scalarized_obj(JVMState* jvms, MachSafePointNode* sfn,
+                                             GrowableArray<ScopeValue*>* objs,
+                                             ObjectValue* ov) const {
+  for (int i = 0; i < jvms->scl_size(); i++) {
+    Node* n = sfn->scalarized_obj(jvms, i);
+    // Other kinds of nodes that we may encounter here, for instance constants
+    // representing values of fields of objects scalarized, aren't relevant for
+    // us, since they don't map to ObjectValue.
+    if (!n->is_SafePointScalarObject()) {
+      continue;
+    }
+
+    ObjectValue* other = (ObjectValue*) sv_for_node_id(objs, n->_idx);
+    if (ov == other) {
+      return true;
+    }
+  }
+  return false;
+}
+
 //--------------------------Process_OopMap_Node--------------------------------
 void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) {
   // Handle special safepoint nodes for synchronization
@@ -1137,7 +1158,10 @@ void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) {
 
         for (int j = 0; j< merge->possible_objects()->length(); j++) {
           ObjectValue* ov = merge->possible_objects()->at(j)->as_ObjectValue();
-          bool is_root = locarray->contains(ov) || exparray->contains(ov) || contains_as_owner(monarray, ov);
+          bool is_root = locarray->contains(ov) ||
+                         exparray->contains(ov) ||
+                         contains_as_owner(monarray, ov) ||
+                         contains_as_scalarized_obj(jvms, sfn, objs, ov);
           ov->set_root(is_root);
         }
       }

src/hotspot/share/opto/output.hpp

@@ -209,6 +209,9 @@ class PhaseOutput : public Phase {
 
   bool starts_bundle(const Node *n) const;
   bool contains_as_owner(GrowableArray<MonitorValue*> *monarray, ObjectValue *ov) const;
+  bool contains_as_scalarized_obj(JVMState* jvms, MachSafePointNode* sfn,
+                                  GrowableArray<ScopeValue*>* objs,
+                                  ObjectValue* ov) const;
 
   // Dump formatted assembly
 #if defined(SUPPORT_OPTO_ASSEMBLY)

src/hotspot/share/opto/superword.cpp

@@ -3717,27 +3717,55 @@ void SuperWord::adjust_pre_loop_limit_to_align_main_loop_vectors() {
   TRACE_ALIGN_VECTOR_NODE(mask_AW);
   TRACE_ALIGN_VECTOR_NODE(adjust_pre_iter);
 
-  // 4: Compute (3a, b):
+  // 4: The computation of the new pre-loop limit could overflow (for 3a) or
+  //    underflow (for 3b) the int range. This is problematic in combination
+  //    with Range Check Elimination (RCE), which determines a "safe" range
+  //    where a RangeCheck will always succeed. RCE adjusts the pre-loop limit
+  //    such that we only enter the main-loop once we have reached the "safe"
+  //    range, and adjusts the main-loop limit so that we exit the main-loop
+  //    before we leave the "safe" range. After RCE, the range of the main-loop
+  //    can only be safely narrowed, and should never be widened. Hence, the
+  //    pre-loop limit can only be increased (for stride > 0), but an add
+  //    overflow might decrease it, or decreased (for stride < 0), but a sub
+  //    underflow might increase it. To prevent that, we perform the Sub / Add
+  //    and Max / Min with long operations.
+  old_limit       = new ConvI2LNode(old_limit);
+  orig_limit      = new ConvI2LNode(orig_limit);
+  adjust_pre_iter = new ConvI2LNode(adjust_pre_iter);
+  phase()->register_new_node(old_limit, pre_ctrl);
+  phase()->register_new_node(orig_limit, pre_ctrl);
+  phase()->register_new_node(adjust_pre_iter, pre_ctrl);
+  TRACE_ALIGN_VECTOR_NODE(old_limit);
+  TRACE_ALIGN_VECTOR_NODE(orig_limit);
+  TRACE_ALIGN_VECTOR_NODE(adjust_pre_iter);
+
+  // 5: Compute (3a, b):
   //    new_limit = old_limit + adjust_pre_iter     (stride > 0)
   //    new_limit = old_limit - adjust_pre_iter     (stride < 0)
+  //
   Node* new_limit = nullptr;
   if (stride < 0) {
-    new_limit = new SubINode(old_limit, adjust_pre_iter);
+    new_limit = new SubLNode(old_limit, adjust_pre_iter);
   } else {
-    new_limit = new AddINode(old_limit, adjust_pre_iter);
+    new_limit = new AddLNode(old_limit, adjust_pre_iter);
   }
   phase()->register_new_node(new_limit, pre_ctrl);
   TRACE_ALIGN_VECTOR_NODE(new_limit);
 
-  // 5: Compute (15a, b):
+  // 6: Compute (15a, b):
   //    Prevent pre-loop from going past the original limit of the loop.
   Node* constrained_limit =
-    (stride > 0) ? (Node*) new MinINode(new_limit, orig_limit)
-                 : (Node*) new MaxINode(new_limit, orig_limit);
+    (stride > 0) ? (Node*) new MinLNode(phase()->C, new_limit, orig_limit)
+                 : (Node*) new MaxLNode(phase()->C, new_limit, orig_limit);
+  phase()->register_new_node(constrained_limit, pre_ctrl);
+  TRACE_ALIGN_VECTOR_NODE(constrained_limit);
+
+  // 7: We know that the result is in the int range, there is never truncation
+  constrained_limit = new ConvL2INode(constrained_limit);
   phase()->register_new_node(constrained_limit, pre_ctrl);
   TRACE_ALIGN_VECTOR_NODE(constrained_limit);
 
-  // 6: Hack the pre-loop limit
+  // 8: Hack the pre-loop limit
   igvn().replace_input_of(pre_opaq, 1, constrained_limit);
 }