8261137: Optimization of Box nodes in uncommon_trap

Co-authored-by: Wu Yan <wuyan34@huawei.com>
Co-authored-by: Ai Jiaming <aijiaming1@huawei.com>
Reviewed-by: kvn, vlivanov, thartmann

Author: 3 people

src/hotspot/share/opto/callGenerator.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2021, 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
@@ -554,6 +554,57 @@ void LateInlineVirtualCallGenerator::do_late_inline() {
   CallGenerator::do_late_inline_helper();
 }
 
+static bool has_non_debug_usages(Node* n) {
+  for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
+    Node* m = n->fast_out(i);
+    if (!m->is_SafePoint()
+        || (m->is_Call() && m->as_Call()->has_non_debug_use(n))) {
+      return true;
+    }
+  }
+  return false;
+}
+
+// delay box in runtime, treat box as a scalarized object
+static void scalarize_debug_usages(CallNode* call, Node* resproj) {
+  GraphKit kit(call->jvms());
+  PhaseGVN& gvn = kit.gvn();
+
+  ProjNode* res = resproj->as_Proj();
+  ciInstanceKlass* klass = call->as_CallStaticJava()->method()->holder();
+  int n_fields = klass->nof_nonstatic_fields();
+  assert(n_fields == 1, "the klass must be an auto-boxing klass");
+
+  for (DUIterator_Last imin, i = res->last_outs(imin); i >= imin;) {
+    SafePointNode* sfpt = res->last_out(i)->as_SafePoint();
+    uint first_ind = sfpt->req() - sfpt->jvms()->scloff();
+    Node* sobj = new SafePointScalarObjectNode(gvn.type(res)->isa_oopptr(),
+#ifdef ASSERT
+                                                call,
+#endif // ASSERT
+                                                first_ind, n_fields, true);
+    sobj->init_req(0, kit.root());
+    sfpt->add_req(call->in(TypeFunc::Parms));
+    sobj = gvn.transform(sobj);
+    JVMState* jvms = sfpt->jvms();
+    jvms->set_endoff(sfpt->req());
+    int start = jvms->debug_start();
+    int end   = jvms->debug_end();
+    int num_edges = sfpt->replace_edges_in_range(res, sobj, start, end, &gvn);
+    i -= num_edges;
+  }
+
+  assert(res->outcnt() == 0, "the box must have no use after replace");
+
+#ifndef PRODUCT
+  if (PrintEliminateAllocations) {
+    tty->print("++++ Eliminated: %d ", call->_idx);
+    call->as_CallStaticJava()->method()->print_short_name(tty);
+    tty->cr();
+  }
+#endif
+}
+
 void CallGenerator::do_late_inline_helper() {
   assert(is_late_inline(), "only late inline allowed");
 
@@ -603,10 +654,23 @@ void CallGenerator::do_late_inline_helper() {
     C->remove_macro_node(call);
   }
 
-  bool result_not_used = (callprojs.resproj == NULL || callprojs.resproj->outcnt() == 0);
-  if (is_pure_call() && result_not_used) {
+  bool result_not_used = false;
+
+  if (is_pure_call()) {
+    if (is_boxing_late_inline() && callprojs.resproj != nullptr) {
+        // replace box node to scalar node only in case it is directly referenced by debug info
+        assert(call->as_CallStaticJava()->is_boxing_method(), "sanity");
+        if (!has_non_debug_usages(callprojs.resproj)) {
+          scalarize_debug_usages(call, callprojs.resproj);
+        }
+    }
+
     // The call is marked as pure (no important side effects), but result isn't used.
     // It's safe to remove the call.
+    result_not_used = (callprojs.resproj == NULL || callprojs.resproj->outcnt() == 0);
+  }
+
+  if (result_not_used) {
     GraphKit kit(call->jvms());
     kit.replace_call(call, C->top(), true);
   } else {
@@ -741,6 +805,8 @@ class LateInlineBoxingCallGenerator : public LateInlineCallGenerator {
     return new_jvms;
   }
 
+  virtual bool is_boxing_late_inline() const { return true; }
+
   virtual CallGenerator* with_call_node(CallNode* call) {
     LateInlineBoxingCallGenerator* cg = new LateInlineBoxingCallGenerator(method(), _inline_cg);
     cg->set_call_node(call->as_CallStaticJava());

src/hotspot/share/opto/callGenerator.hpp

@@ -74,6 +74,7 @@ class CallGenerator : public ResourceObj {
   virtual bool      is_late_inline() const         { return false; }
   // same but for method handle calls
   virtual bool      is_mh_late_inline() const      { return false; }
+  virtual bool      is_boxing_late_inline() const  { return false; }
   virtual bool      is_string_late_inline() const  { return false; }
   virtual bool      is_virtual_late_inline() const { return false; }
 

src/hotspot/share/opto/callnode.cpp

@@ -1515,17 +1515,27 @@ void SafePointNode::disconnect_from_root(PhaseIterGVN *igvn) {
 
 SafePointScalarObjectNode::SafePointScalarObjectNode(const TypeOopPtr* tp,
 #ifdef ASSERT
-                                                     AllocateNode* alloc,
+                                                     Node* alloc,
 #endif
                                                      uint first_index,
-                                                     uint n_fields) :
+                                                     uint n_fields,
+                                                     bool is_auto_box) :
   TypeNode(tp, 1), // 1 control input -- seems required.  Get from root.
   _first_index(first_index),
-  _n_fields(n_fields)
+  _n_fields(n_fields),
+  _is_auto_box(is_auto_box)
 #ifdef ASSERT
   , _alloc(alloc)
 #endif
 {
+#ifdef ASSERT
+  if (!alloc->is_Allocate()
+      && !(alloc->Opcode() == Op_VectorBox)
+      && (!alloc->is_CallStaticJava() || !alloc->as_CallStaticJava()->is_boxing_method())) {
+    alloc->dump();
+    assert(false, "unexpected call node");
+  }
+#endif
   init_class_id(Class_SafePointScalarObject);
 }
 

src/hotspot/share/opto/callnode.hpp

@@ -501,7 +501,8 @@ class SafePointScalarObjectNode: public TypeNode {
                      // states of the scalarized object fields are collected.
                      // It is relative to the last (youngest) jvms->_scloff.
   uint _n_fields;    // Number of non-static fields of the scalarized object.
-  DEBUG_ONLY(AllocateNode* _alloc;)
+  bool _is_auto_box; // True if the scalarized object is an auto box.
+  DEBUG_ONLY(Node* _alloc;)
 
   virtual uint hash() const ; // { return NO_HASH; }
   virtual bool cmp( const Node &n ) const;
@@ -511,9 +512,9 @@ class SafePointScalarObjectNode: public TypeNode {
 public:
   SafePointScalarObjectNode(const TypeOopPtr* tp,
 #ifdef ASSERT
-                            AllocateNode* alloc,
+                            Node* alloc,
 #endif
-                            uint first_index, uint n_fields);
+                            uint first_index, uint n_fields, bool is_auto_box = false);
   virtual int Opcode() const;
   virtual uint           ideal_reg() const;
   virtual const RegMask &in_RegMask(uint) const;
@@ -526,8 +527,9 @@ class SafePointScalarObjectNode: public TypeNode {
   }
   uint n_fields()    const { return _n_fields; }
 
+  bool is_auto_box() const { return _is_auto_box; }
 #ifdef ASSERT
-  AllocateNode* alloc() const { return _alloc; }
+  Node* alloc() const { return _alloc; }
 #endif
 
   virtual uint size_of() const { return sizeof(*this); }

src/hotspot/share/opto/output.cpp

@@ -826,8 +826,9 @@ void PhaseOutput::FillLocArray( int idx, MachSafePointNode* sfpt, Node *local,
       ciKlass* cik = t->is_oopptr()->klass();
       assert(cik->is_instance_klass() ||
              cik->is_array_klass(), "Not supported allocation.");
-      sv = new ObjectValue(spobj->_idx,
-                           new ConstantOopWriteValue(cik->java_mirror()->constant_encoding()));
+      ScopeValue* klass_sv = new ConstantOopWriteValue(cik->java_mirror()->constant_encoding());
+      sv = spobj->is_auto_box() ? new AutoBoxObjectValue(spobj->_idx, klass_sv)
+                                    : new ObjectValue(spobj->_idx, klass_sv);
       set_sv_for_object_node(objs, sv);
 
       uint first_ind = spobj->first_index(sfpt->jvms());
@@ -1099,8 +1100,9 @@ void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) {
           ciKlass* cik = t->is_oopptr()->klass();
           assert(cik->is_instance_klass() ||
                  cik->is_array_klass(), "Not supported allocation.");
-          ObjectValue* sv = new ObjectValue(spobj->_idx,
-                                            new ConstantOopWriteValue(cik->java_mirror()->constant_encoding()));
+          ScopeValue* klass_sv = new ConstantOopWriteValue(cik->java_mirror()->constant_encoding());
+          ObjectValue* sv = spobj->is_auto_box() ? new AutoBoxObjectValue(spobj->_idx, klass_sv)
+                                        : new ObjectValue(spobj->_idx, klass_sv);
           PhaseOutput::set_sv_for_object_node(objs, sv);
 
           uint first_ind = spobj->first_index(youngest_jvms);

src/hotspot/share/opto/vector.cpp

@@ -248,7 +248,7 @@ void PhaseVector::scalarize_vbox_node(VectorBoxNode* vec_box) {
     uint first_ind = (sfpt->req() - sfpt->jvms()->scloff());
     Node* sobj = new SafePointScalarObjectNode(vec_box->box_type(),
 #ifdef ASSERT
-                                               NULL,
+                                               vec_box,
 #endif // ASSERT
                                                first_ind, n_fields);
     sobj->init_req(0, C->root());

src/hotspot/share/runtime/deoptimization.cpp

@@ -898,7 +898,7 @@ Deoptimization::DeoptAction Deoptimization::_unloaded_action
 
 
 
-#if INCLUDE_JVMCI || INCLUDE_AOT
+#if COMPILER2_OR_JVMCI || INCLUDE_AOT
 template<typename CacheType>
 class BoxCacheBase : public CHeapObj<mtCompiler> {
 protected:
@@ -1026,7 +1026,7 @@ oop Deoptimization::get_cached_box(AutoBoxObjectValue* bv, frame* fr, RegisterMa
    }
    return NULL;
 }
-#endif // INCLUDE_JVMCI || INCLUDE_AOT
+#endif // COMPILER2_OR_JVMCI || INCLUDE_AOT
 
 #if COMPILER2_OR_JVMCI
 bool Deoptimization::realloc_objects(JavaThread* thread, frame* fr, RegisterMap* reg_map, GrowableArray<ScopeValue*>* objects, TRAPS) {
@@ -1045,17 +1045,16 @@ bool Deoptimization::realloc_objects(JavaThread* thread, frame* fr, RegisterMap*
     oop obj = NULL;
 
     if (k->is_instance_klass()) {
-#if INCLUDE_JVMCI || INCLUDE_AOT
-      CompiledMethod* cm = fr->cb()->as_compiled_method_or_null();
-      if (cm->is_compiled_by_jvmci() && sv->is_auto_box()) {
+#if COMPILER2_OR_JVMCI || INCLUDE_AOT
+      if (sv->is_auto_box()) {
         AutoBoxObjectValue* abv = (AutoBoxObjectValue*) sv;
         obj = get_cached_box(abv, fr, reg_map, THREAD);
         if (obj != NULL) {
           // Set the flag to indicate the box came from a cache, so that we can skip the field reassignment for it.
           abv->set_cached(true);
         }
       }
-#endif // INCLUDE_JVMCI || INCLUDE_AOT
+#endif // COMPILER2_OR_JVMCI || INCLUDE_AOT
       InstanceKlass* ik = InstanceKlass::cast(k);
       if (obj == NULL) {
 #ifdef COMPILER2
@@ -1397,12 +1396,12 @@ void Deoptimization::reassign_fields(frame* fr, RegisterMap* reg_map, GrowableAr
     if (obj.is_null()) {
       continue;
     }
-#if INCLUDE_JVMCI || INCLUDE_AOT
+#if COMPILER2_OR_JVMCI || INCLUDE_AOT
     // Don't reassign fields of boxes that came from a cache. Caches may be in CDS.
     if (sv->is_auto_box() && ((AutoBoxObjectValue*) sv)->is_cached()) {
       continue;
     }
-#endif // INCLUDE_JVMCI || INCLUDE_AOT
+#endif // COMPILER2_OR_JVMCI || INCLUDE_AOT
 #ifdef COMPILER2
     if (EnableVectorSupport && VectorSupport::is_vector(k)) {
       assert(sv->field_size() == 1, "%s not a vector", k->name()->as_C_string());

src/hotspot/share/runtime/deoptimization.hpp

@@ -164,9 +164,10 @@ class Deoptimization : AllStatic {
 
 #if INCLUDE_JVMCI
   static address deoptimize_for_missing_exception_handler(CompiledMethod* cm);
-  static oop get_cached_box(AutoBoxObjectValue* bv, frame* fr, RegisterMap* reg_map, TRAPS);
 #endif
 
+  static oop get_cached_box(AutoBoxObjectValue* bv, frame* fr, RegisterMap* reg_map, TRAPS);
+
   private:
   // Does the actual work for deoptimizing a single frame
   static void deoptimize_single_frame(JavaThread* thread, frame fr, DeoptReason reason);

test/hotspot/jtreg/compiler/eliminateAutobox/TestEliminateBoxInDebugInfo.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2021, Huawei Technologies Co., Ltd. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8261137
+ * @requires vm.debug == true & vm.compiler2.enabled
+ * @summary Verify that box object is scalarized in case it is directly referenced by debug info.
+ * @library /test/lib
+ *
+ * @run main/othervm compiler.c2.TestEliminateBoxInDebugInfo
+ */
+package compiler.c2;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import jdk.test.lib.Platform;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+
+import jdk.test.lib.Asserts;
+
+public class TestEliminateBoxInDebugInfo {
+    public static void runTest() throws Exception {
+        final String[] arguments = {
+            "-XX:CompileCommand=compileonly,compiler/c2/TestEliminateBoxInDebugInfo$Test.foo",
+            "-XX:CompileCommand=dontinline,compiler/c2/TestEliminateBoxInDebugInfo$Test.black",
+            "-Xbatch",
+            "-XX:+PrintEliminateAllocations",
+            Test.class.getName()
+            };
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(arguments);
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        System.out.println(output.getStdout());
+        String pattern = ".*Eliminated.*";
+        Pattern r = Pattern.compile(pattern);
+        Matcher m = r.matcher(output.getStdout());
+        if (!m.find()) {
+            throw new RuntimeException("Could not find Elimination output");
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        runTest();
+    }
+
+    static class Test {
+        public static void main(String[] args) throws Exception {
+            // warmup
+            for(int i = 0; i < 100000; i++) {
+               foo(1000 + (i % 1000));
+            }
+        }
+
+        public static int foo(int value) {
+            Integer ii = Integer.valueOf(value);
+            int sum = 0;
+            if (value > 999) {
+                sum += ii.intValue();
+            }
+            black();
+            return sum;
+        }
+
+        public static void black() {}
+    }
+}

test/hotspot/jtreg/compiler/eliminateAutobox/TestIdentityWithEliminateBoxInDebugInfo.java

@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2021, Huawei Technologies Co., Ltd. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8261137
+ * @requires vm.flavor == "server"
+ * @summary Verify that box object identity matches after deoptimization When it is eliminated.
+ * @library /test/lib
+ *
+ * @run main/othervm -Xbatch compiler.c2.TestIdentityWithEliminateBoxInDebugInfo
+ */
+package compiler.c2;
+
+import jdk.test.lib.Asserts;
+
+public class TestIdentityWithEliminateBoxInDebugInfo {
+    interface TestF {
+        void apply(boolean condition);
+    }
+
+    public static void helper(TestF f) {
+      // warmup
+      for(int i = 0; i < 100000; i++) {
+        f.apply(true);
+      }
+      // deoptimize
+      f.apply(false);
+    }
+
+    public static void runTest() throws Exception {
+        helper((c) -> {
+            Integer a = Integer.valueOf(42);
+            Integer b = Integer.valueOf(-42);
+            if (!c) {
+                Asserts.assertTrue(a == Integer.valueOf(42));
+                Asserts.assertTrue(b == Integer.valueOf(-42));
+            }
+        });
+
+        helper((c) -> {
+            long highBitsOnly = 2L << 40;
+            Long a = Long.valueOf(42L);
+            Long b = Long.valueOf(-42L);
+            Long h = Long.valueOf(highBitsOnly);
+            if (!c) {
+                Asserts.assertTrue(a == Long.valueOf(42L));
+                Asserts.assertTrue(b == Long.valueOf(-42L));
+                Asserts.assertFalse(h == Long.valueOf(highBitsOnly));
+            }
+        });
+
+        helper((c) -> {
+            Character a = Character.valueOf('a');
+            Character b = Character.valueOf('Z');
+            if (!c) {
+                Asserts.assertTrue(a == Character.valueOf('a'));
+                Asserts.assertTrue(b == Character.valueOf('Z'));
+            }
+        });
+
+        helper((c) -> {
+            Short a = Short.valueOf((short)42);
+            Short b = Short.valueOf((short)-42);
+            if (!c) {
+                Asserts.assertTrue(a == Short.valueOf((short)42));
+                Asserts.assertTrue(b == Short.valueOf((short)-42));
+            }
+        });
+
+        helper((c) -> {
+            Byte a = Byte.valueOf((byte)42);
+            Byte b = Byte.valueOf((byte)-42);
+            if (!c) {
+                Asserts.assertTrue(a == Byte.valueOf((byte)42));
+                Asserts.assertTrue(b == Byte.valueOf((byte)-42));
+            }
+        });
+
+        helper((c) -> {
+            Boolean a = Boolean.valueOf(true);
+            Boolean b = Boolean.valueOf(false);
+            if (!c) {
+                Asserts.assertTrue(a == Boolean.valueOf(true));
+                Asserts.assertTrue(b == Boolean.valueOf(false));
+            }
+        });
+    }
+
+    public static void main(String[] args) throws Exception {
+        runTest();
+    }
+}