8380191: [lworld] [BACKOUT] OptimizePtrCompare is too conservative

TobiHartmann · TobiHartmann · commit 2701ce240bee · 2026-03-16T16:34:59.000Z
Reviewed-by: dcubed
diff --git a/src/hotspot/share/opto/escape.cpp b/src/hotspot/share/opto/escape.cpp
@@ -1586,39 +1586,6 @@ void ConnectionGraph::add_objload_to_connection_graph(Node *n, Unique_Node_List
   }
 }
 
-void ConnectionGraph::add_proj(Node* n, Unique_Node_List* delayed_worklist) {
-  if (n->as_Proj()->_con == TypeFunc::Parms && n->in(0)->is_Call() && n->in(0)->as_Call()->returns_pointer()) {
-    add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), delayed_worklist);
-  } else if (n->in(0)->is_LoadFlat()) {
-    // Treat LoadFlat outputs similar to a call return value
-    add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), delayed_worklist);
-  } else if (n->as_Proj()->_con >= TypeFunc::Parms && n->in(0)->is_Call() && n->bottom_type()->isa_ptr()) {
-    CallNode* call = n->in(0)->as_Call();
-    assert(call->tf()->returns_inline_type_as_fields(), "");
-    ciMethod* meth = n->in(0)->as_CallJava()->method();
-    BCEscapeAnalyzer* call_analyzer = (meth != nullptr) ? meth->get_bcea() : nullptr;
-    const TypeTuple* d = call->tf()->domain_sig();
-    uint i = d->cnt();
-    if (call_analyzer != nullptr) {
-      for (i = TypeFunc::Parms; i < d->cnt(); i++) {
-        if (d->field_at(i)->isa_ptr() != nullptr &&
-            call_analyzer->is_arg_returned(i - TypeFunc::Parms)) {
-          break;
-        }
-      }
-    }
-    if (n->as_Proj()->_con == TypeFunc::Parms || i >= d->cnt() || !scalarized_arg_with_compatible_return(call->as_CallJava(), i)) {
-      // either:
-      // - not an argument returned
-      // - the returned buffer for a returned scalarized argument
-      // - an incompatible  returned scalarized argument
-      add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), delayed_worklist);
-    } else {
-      add_local_var(n, PointsToNode::NoEscape);
-    }
-  }
-}
-
 // Populate Connection Graph with PointsTo nodes and create simple
 // connection graph edges.
 void ConnectionGraph::add_node_to_connection_graph(Node *n, Unique_Node_List *delayed_worklist) {
@@ -1782,7 +1749,15 @@ void ConnectionGraph::add_node_to_connection_graph(Node *n, Unique_Node_List *de
       break;
     case Op_Proj: {
       // we are only interested in the oop result projection from a call
-      add_proj(n, delayed_worklist);
+      if (n->as_Proj()->_con >= TypeFunc::Parms && n->in(0)->is_Call() &&
+          (n->in(0)->as_Call()->returns_pointer() || n->bottom_type()->isa_ptr())) {
+        assert((n->as_Proj()->_con == TypeFunc::Parms && n->in(0)->as_Call()->returns_pointer()) ||
+               n->in(0)->as_Call()->tf()->returns_inline_type_as_fields(), "what kind of oop return is it?");
+        add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), delayed_worklist);
+      } else if (n->as_Proj()->_con >= TypeFunc::Parms && n->in(0)->is_LoadFlat() && igvn->type(n)->isa_ptr()) {
+        // Treat LoadFlat outputs similar to a call return value
+        add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), delayed_worklist);
+      }
       break;
     }
     case Op_Rethrow: // Exception object escapes
@@ -1952,7 +1927,15 @@ void ConnectionGraph::add_final_edges(Node *n) {
       break;
     }
     case Op_Proj: {
-      add_proj(n, nullptr);
+      if (n->in(0)->is_Call()) {
+        // we are only interested in the oop result projection from a call
+        assert((n->as_Proj()->_con == TypeFunc::Parms && n->in(0)->as_Call()->returns_pointer()) ||
+              n->in(0)->as_Call()->tf()->returns_inline_type_as_fields(), "what kind of oop return is it?");
+        add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), nullptr);
+      } else if (n->in(0)->is_LoadFlat()) {
+        // Treat LoadFlat outputs similar to a call return value
+        add_local_var_and_edge(n, PointsToNode::NoEscape, n->in(0), nullptr);
+      }
       break;
     }
     case Op_Rethrow: // Exception object escapes
@@ -2146,8 +2129,6 @@ class DomainIterator : public StackObj {
   uint _i_domain_cc;
   int _i_sig_cc;
   uint _depth;
-  uint _first_field_pos;
-  const bool _is_static;
 
   void next_helper() {
     if (_sig_cc == nullptr) {
@@ -2156,22 +2137,14 @@ class DomainIterator : public StackObj {
     BasicType prev_bt = _i_sig_cc > 0 ? _sig_cc->at(_i_sig_cc-1)._bt : T_ILLEGAL;
     while (_i_sig_cc < _sig_cc->length()) {
       BasicType bt = _sig_cc->at(_i_sig_cc)._bt;
-      assert(bt != T_VOID || _sig_cc->at(_i_sig_cc-1)._bt == prev_bt, "incorrect prev bt");
+      assert(bt != T_VOID || _sig_cc->at(_i_sig_cc-1)._bt == prev_bt, "");
       if (bt == T_METADATA) {
-        if (_depth == 0) {
-          _first_field_pos = _i_domain_cc;
-        }
         _depth++;
       } else if (bt == T_VOID && (prev_bt != T_LONG && prev_bt != T_DOUBLE)) {
         _depth--;
         if (_depth == 0) {
           _i_domain++;
         }
-      } else if (bt == T_BOOLEAN && prev_bt == T_METADATA && (_is_static || _i_domain > 0) && _sig_cc->at(_i_sig_cc)._offset == -1) {
-        assert(_sig_cc->at(_i_sig_cc)._null_marker, "null marker expected right after T_METADATA");
-        assert(_depth == 1, "only root value null marker");
-        _i_domain_cc++;
-        _first_field_pos = _i_domain_cc;
       } else {
         return;
       }
@@ -2189,9 +2162,7 @@ class DomainIterator : public StackObj {
     _i_domain(TypeFunc::Parms),
     _i_domain_cc(TypeFunc::Parms),
     _i_sig_cc(0),
-    _depth(0),
-    _first_field_pos(0),
-    _is_static(call->method()->is_static()) {
+    _depth(0) {
     next_helper();
   }
 
@@ -2226,11 +2197,6 @@ class DomainIterator : public StackObj {
   const Type* current_domain_cc() const {
     return _domain_cc->field_at(_i_domain_cc);
   }
-
-  uint first_field_pos() const {
-    assert(_first_field_pos >= TypeFunc::Parms, "not yet updated?");
-    return _first_field_pos;
-  }
 };
 
 void ConnectionGraph::add_call_node(CallNode* call) {
@@ -2342,25 +2308,21 @@ void ConnectionGraph::add_call_node(CallNode* call) {
         add_java_object(call, PointsToNode::NoEscape);
         set_not_scalar_replaceable(ptnode_adr(call_idx) NOT_PRODUCT(COMMA "is result of call"));
       } else {
+        bool ret_arg = false;
         // Determine whether any arguments are returned.
-        const TypeTuple* d = call->tf()->domain_sig();
-        uint i = TypeFunc::Parms;
-        for (; i < d->cnt(); i++) {
-          if (d->field_at(i)->isa_ptr() != nullptr &&
-              call_analyzer->is_arg_returned(i - TypeFunc::Parms)) {
+        for (DomainIterator di(call->as_CallJava()); di.has_next(); di.next()) {
+          uint arg = di.i_domain() - TypeFunc::Parms;
+          if (di.current_domain_cc()->isa_ptr() != nullptr &&
+              call_analyzer->is_arg_returned(arg) &&
+              !meth->is_scalarized_arg(arg)) {
+            ret_arg = true;
             break;
           }
         }
-        // For non scalarized argument/return: add_proj() adds an edge between the return projection and the call,
-        // process_call_arguments() adds an edge between the call and the argument
-        // For scalarized argument/return: process_call_arguments() adds an edge between a call projection for a field
-        // and the argument input to the call for that field. An edge is added between the projection for the returned
-        // buffer and the call.
-        if (i < d->cnt() && !scalarized_arg_with_compatible_return(call->as_CallJava(), i)) {
-          // returns non scalarized argument or incompatible return
+        if (ret_arg) {
           add_local_var(call, PointsToNode::ArgEscape);
         } else {
-          // Returns unknown object or scalarized argument being returned
+          // Returns unknown object.
           map_ideal_node(call, phantom_obj);
         }
       }
@@ -2373,16 +2335,6 @@ void ConnectionGraph::add_call_node(CallNode* call) {
   }
 }
 
-// Check that the return type is compatible with the type of the argument being returned i.e. that there's no cast that
-// fails in the method
-bool ConnectionGraph::scalarized_arg_with_compatible_return(CallJavaNode* call, uint k) {
-  ciMethod* meth = call->method();
-  BCEscapeAnalyzer* call_analyzer = meth->get_bcea();
-  assert(call_analyzer->is_arg_returned(k - TypeFunc::Parms), "only for returned arg");
-  return meth->is_scalarized_arg(k - TypeFunc::Parms) &&
-         call->tf()->domain_sig()->field_at(k)->meet(TypePtr::NULL_PTR)->higher_equal(call->tf()->range_sig()->field_at(TypeFunc::Parms));
-}
-
 void ConnectionGraph::process_call_arguments(CallNode *call) {
     bool is_arraycopy = false;
     switch (call->Opcode()) {
@@ -2575,27 +2527,11 @@ void ConnectionGraph::process_call_arguments(CallNode *call) {
           const Type* at = di.current_domain_cc();
           Node* arg = call->in(di.i_domain_cc());
           PointsToNode* arg_ptn = ptnode_adr(arg->_idx);
-          assert(!call_analyzer->is_arg_returned(k) || !scalarized_arg_with_compatible_return(call->as_CallJava(), di.i_domain()) ||
-                 call->proj_out_or_null(di.i_domain_cc() - di.first_field_pos() + TypeFunc::Parms + 1) == nullptr ||
-                 _igvn->type(call->proj_out_or_null(di.i_domain_cc() - di.first_field_pos() + TypeFunc::Parms + 1)) == at,
-                 "scalarized return and scalarized argument should match");
           if (at->isa_ptr() != nullptr &&
-              call_analyzer->is_arg_returned(k)) {
+              call_analyzer->is_arg_returned(k) &&
+              !meth->is_scalarized_arg(k)) {
             // The call returns arguments.
-            if (scalarized_arg_with_compatible_return(call->as_CallJava(), di.i_domain())) {
-              ProjNode* res_proj = call->proj_out_or_null(di.i_domain_cc() - di.first_field_pos() + TypeFunc::Parms + 1);
-              if (res_proj != nullptr) {
-                assert(_igvn->type(res_proj)->isa_ptr(), "scalarized return and scalarized argument should match");
-                if (res_proj->_con != TypeFunc::Parms) {
-                  // add an edge between the result projection for a field and the argument projection for the same argument field
-                  PointsToNode* proj_ptn = ptnode_adr(res_proj->_idx);
-                  add_edge(proj_ptn, arg_ptn);
-                  if (!call_analyzer->is_return_local()) {
-                    add_edge(proj_ptn, phantom_obj);
-                  }
-                }
-              }
-            } else if (call_ptn != nullptr) { // Is call's result used?
+            if (call_ptn != nullptr) { // Is call's result used?
               assert(call_ptn->is_LocalVar(), "node should be registered");
               assert(arg_ptn != nullptr, "node should be registered");
               add_edge(call_ptn, arg_ptn);
diff --git a/src/hotspot/share/opto/escape.hpp b/src/hotspot/share/opto/escape.hpp
@@ -665,8 +665,6 @@ class ConnectionGraph: public ArenaObj {
   // Utility function for nodes that load an object
   void add_objload_to_connection_graph(Node* n, Unique_Node_List* delayed_worklist);
 
-  void add_proj(Node* n, Unique_Node_List* delayed_worklist);
-
   // Add LocalVar node and edge if possible
   void add_local_var_and_edge(Node* n, PointsToNode::EscapeState es, Node* to,
                               Unique_Node_List *delayed_worklist) {
@@ -692,8 +690,6 @@ class ConnectionGraph: public ArenaObj {
   void add_to_congraph_unsafe_access(Node* n, uint opcode, Unique_Node_List* delayed_worklist);
   bool add_final_edges_unsafe_access(Node* n, uint opcode);
 
-  bool scalarized_arg_with_compatible_return(CallJavaNode* call, uint k);
-
 #ifndef PRODUCT
   static int _no_escape_counter;
   static int _arg_escape_counter;
diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java
@@ -653,11 +653,6 @@ public static void callLeafNoFpOfMethodNodes(String irNodePlaceholder, String ca
         beforeMatchingNameRegex(CMP_N, "CmpN");
     }
 
-    public static final String CMP_P_OR_N = PREFIX + "CMP_P_OR_N" + POSTFIX;
-    static {
-        beforeMatchingNameRegex(CMP_P_OR_N, "Cmp(P|N)");
-    }
-
     public static final String CMP_LT_MASK = PREFIX + "CMP_LT_MASK" + POSTFIX;
     static {
         beforeMatchingNameRegex(CMP_LT_MASK, "CmpLTMask");
diff --git a/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestOptimizePtrCompare.java b/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestOptimizePtrCompare.java
