6589834: deoptimization problem with -XX:+DeoptimizeALot

Summary: Relocate the stack pointer adjustment to where uncommon_trap is actually inserted for new_array.
Reviewed-by: kvn, jrose

6 files changed, 139 insertions, 17 deletions

diff --git a/src/share/vm/opto/graphKit.cpp b/src/share/vm/opto/graphKit.cpp
index c37e7c85d..db27bbada 100644
--- a/src/share/vm/opto/graphKit.cpp
+++ b/src/share/vm/opto/graphKit.cpp
@@ -2980,6 +2980,7 @@ Node* GraphKit::new_instance(Node* klass_node,
 // See comments on new_instance for the meaning of the other arguments.
 Node* GraphKit::new_array(Node* klass_node,     // array klass (maybe variable)
                           Node* length,         // number of array elements
+                          int   nargs,          // number of arguments to push back for uncommon trap
                           bool raw_mem_only,    // affect only raw memory
                           Node* *return_size_val) {
   jint  layout_con = Klass::_lh_neutral_value;
@@ -2995,6 +2996,7 @@ Node* GraphKit::new_array(Node* klass_node,     // array klass (maybe variable)
     Node* cmp_lh = _gvn.transform( new(C, 3) CmpINode(layout_val, intcon(layout_con)) );
     Node* bol_lh = _gvn.transform( new(C, 2) BoolNode(cmp_lh, BoolTest::eq) );
     { BuildCutout unless(this, bol_lh, PROB_MAX);
+      _sp += nargs;
       uncommon_trap(Deoptimization::Reason_class_check,
                     Deoptimization::Action_maybe_recompile);
     }
diff --git a/src/share/vm/opto/graphKit.hpp b/src/share/vm/opto/graphKit.hpp
index 7445aa277..f47bc49a7 100644
--- a/src/share/vm/opto/graphKit.hpp
+++ b/src/share/vm/opto/graphKit.hpp
@@ -699,7 +699,7 @@ class GraphKit : public Phase {
                      Node* slow_test = NULL,
                      bool raw_mem_only = false,
                      Node* *return_size_val = NULL);
-  Node* new_array(Node* klass_node, Node* count_val,
+  Node* new_array(Node* klass_node, Node* count_val, int nargs,
                   bool raw_mem_only = false, Node* *return_size_val = NULL);
 
   // Handy for making control flow
diff --git a/src/share/vm/opto/library_call.cpp b/src/share/vm/opto/library_call.cpp
index a57333fd5..c51344d4d 100644
--- a/src/share/vm/opto/library_call.cpp
+++ b/src/share/vm/opto/library_call.cpp
@@ -3055,9 +3055,7 @@ bool LibraryCallKit::inline_native_newArray() {
     // Normal case:  The array type has been cached in the java.lang.Class.
     // The following call works fine even if the array type is polymorphic.
     // It could be a dynamic mix of int[], boolean[], Object[], etc.
-    _sp += nargs;  // set original stack for use by uncommon_trap
-    Node* obj = new_array(klass_node, count_val);
-    _sp -= nargs;
+    Node* obj = new_array(klass_node, count_val, nargs);
     result_reg->init_req(_normal_path, control());
     result_val->init_req(_normal_path, obj);
     result_io ->init_req(_normal_path, i_o());
@@ -3179,9 +3177,7 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {
     Node* orig_tail = _gvn.transform( new(C, 3) SubINode(orig_length, start) );
     Node* moved = generate_min_max(vmIntrinsics::_min, orig_tail, length);
 
-    _sp += nargs;  // set original stack for use by uncommon_trap
-    Node* newcopy = new_array(klass_node, length);
-    _sp -= nargs;
+    Node* newcopy = new_array(klass_node, length, nargs);
 
     // Generate a direct call to the right arraycopy function(s).
     // We know the copy is disjoint but we might not know if the
@@ -3903,10 +3899,8 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) {
     set_control(array_ctl);
     Node* obj_length = load_array_length(obj);
     Node* obj_size = NULL;
-    _sp += nargs;  // set original stack for use by uncommon_trap
-    Node* alloc_obj = new_array(obj_klass, obj_length,
+    Node* alloc_obj = new_array(obj_klass, obj_length, nargs,
                                 raw_mem_only, &obj_size);
-    _sp -= nargs;
     assert(obj_size != NULL, "");
     Node* raw_obj = alloc_obj->in(1);
     assert(raw_obj->is_Proj() && raw_obj->in(0)->is_Allocate(), "");
diff --git a/src/share/vm/opto/parse.hpp b/src/share/vm/opto/parse.hpp
index e00dfca6a..7120c51a1 100644
--- a/src/share/vm/opto/parse.hpp
+++ b/src/share/vm/opto/parse.hpp
@@ -476,7 +476,7 @@ class Parse : public GraphKit {
   void do_newarray(BasicType elemtype);
   void do_anewarray();
   void do_multianewarray();
-  Node* expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, int ndimensions);
+  Node* expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, int ndimensions, int nargs);
 
   // implementation of jsr/ret
   void do_jsr();
diff --git a/src/share/vm/opto/parse3.cpp b/src/share/vm/opto/parse3.cpp
index 2a603de28..b3e95ad66 100644
--- a/src/share/vm/opto/parse3.cpp
+++ b/src/share/vm/opto/parse3.cpp
@@ -335,7 +335,7 @@ void Parse::do_anewarray() {
 
   const TypeKlassPtr* array_klass_type = TypeKlassPtr::make(array_klass);
   Node* count_val = pop();
-  Node* obj = new_array(makecon(array_klass_type), count_val);
+  Node* obj = new_array(makecon(array_klass_type), count_val, 1);
   push(obj);
 }
 
@@ -345,17 +345,17 @@ void Parse::do_newarray(BasicType elem_type) {
 
   Node*   count_val = pop();
   const TypeKlassPtr* array_klass = TypeKlassPtr::make(ciTypeArrayKlass::make(elem_type));
-  Node*   obj = new_array(makecon(array_klass), count_val);
+  Node*   obj = new_array(makecon(array_klass), count_val, 1);
   // Push resultant oop onto stack
   push(obj);
 }
 
 // Expand simple expressions like new int[3][5] and new Object[2][nonConLen].
 // Also handle the degenerate 1-dimensional case of anewarray.
-Node* Parse::expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, int ndimensions) {
+Node* Parse::expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, int ndimensions, int nargs) {
   Node* length = lengths[0];
   assert(length != NULL, "");
-  Node* array = new_array(makecon(TypeKlassPtr::make(array_klass)), length);
+  Node* array = new_array(makecon(TypeKlassPtr::make(array_klass)), length, nargs);
   if (ndimensions > 1) {
     jint length_con = find_int_con(length, -1);
     guarantee(length_con >= 0, "non-constant multianewarray");
@@ -364,7 +364,7 @@ Node* Parse::expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, in
     const Type*    elemtype = _gvn.type(array)->is_aryptr()->elem();
     const intptr_t header   = arrayOopDesc::base_offset_in_bytes(T_OBJECT);
     for (jint i = 0; i < length_con; i++) {
-      Node*    elem   = expand_multianewarray(array_klass_1, &lengths[1], ndimensions-1);
+      Node*    elem   = expand_multianewarray(array_klass_1, &lengths[1], ndimensions-1, nargs);
       intptr_t offset = header + ((intptr_t)i << LogBytesPerHeapOop);
       Node*    eaddr  = basic_plus_adr(array, offset);
       store_oop_to_array(control(), array, eaddr, adr_type, elem, elemtype, T_OBJECT);
@@ -419,7 +419,7 @@ void Parse::do_multianewarray() {
   // Can use multianewarray instead of [a]newarray if only one dimension,
   // or if all non-final dimensions are small constants.
   if (expand_count == 1 || (1 <= expand_count && expand_count <= expand_limit)) {
-    Node* obj = expand_multianewarray(array_klass, &length[0], ndimensions);
+    Node* obj = expand_multianewarray(array_klass, &length[0], ndimensions, ndimensions);
     push(obj);
     return;
   }
diff --git a/test/compiler/6589834/Test_ia32.java b/test/compiler/6589834/Test_ia32.java
new file mode 100644
index 000000000..531c91023
--- /dev/null
+++ b/test/compiler/6589834/Test_ia32.java
@@ -0,0 +1,126 @@
+/*
+ * Copyright 2009 Sun Microsystems, Inc.  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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/**
+ * @test
+ * @bug 6589834
+ * @summary: deoptimization problem with -XX:+DeoptimizeALot
+ *
+ * @run main/othervm -server Test_ia32
+ */
+
+/***************************************************************************************
+NOTE: The bug shows up (with several "Bug!" message) even without the
+      flag -XX:+DeoptimizeALot. In a debug build, you may want to try
+      the flags -XX:+VerifyStack and -XX:+DeoptimizeALot to get more information.
+****************************************************************************************/
+import java.lang.reflect.Constructor;
+
+public class Test_ia32 {
+
+    public static int NUM_THREADS = 100;
+
+    public static int CLONE_LENGTH = 1000;
+
+    public static void main(String[] args) throws InterruptedException, ClassNotFoundException {
+
+        Reflector[] threads = new Reflector[NUM_THREADS];
+        for (int i = 0; i < threads.length; i++) {
+            threads[i] = new Reflector();
+            threads[i].start();
+        }
+
+        System.out.println("Give Reflector.run() some time to compile...");
+        Thread.sleep(5000);
+
+        System.out.println("Load RMISecurityException causing run() deoptimization");
+        ClassLoader.getSystemClassLoader().loadClass("java.rmi.RMISecurityException");
+
+        for (Reflector thread : threads)
+            thread.requestStop();
+
+        for (Reflector thread : threads)
+            try {
+                thread.join();
+            } catch (InterruptedException e) {
+                System.out.println(e);
+            }
+
+    }
+
+}
+
+class Reflector extends Thread {
+
+    volatile boolean _doSpin = true;
+
+    Test_ia32[] _tests;
+
+    Reflector() {
+        _tests = new Test_ia32[Test_ia32.CLONE_LENGTH];
+        for (int i = 0; i < _tests.length; i++) {
+            _tests[i] = new Test_ia32();
+        }
+    }
+
+    static int g(int i1, int i2, Test_ia32[] arr, int i3, int i4) {
+
+        if (!(i1==1 && i2==2 && i3==3 && i4==4)) {
+            System.out.println("Bug!");
+        }
+
+        return arr.length;
+    }
+
+    static int f(Test_ia32[] arr) {
+        return g(1, 2, arr.clone(), 3, 4);
+    }
+
+    @Override
+    public void run() {
+        Constructor[] ctrs = null;
+        Class<Test_ia32> klass = Test_ia32.class;
+        try {
+            ctrs = klass.getConstructors();
+        } catch (SecurityException e) {
+            System.out.println(e);
+        }
+
+        try {
+            while (_doSpin) {
+                if (f(_tests) < 0)
+                    System.out.println("return value usage");
+            }
+        } catch (NullPointerException e) {
+            e.printStackTrace();
+        }
+
+        System.out.println(this + " - stopped.");
+    }
+
+    public void requestStop() {
+        System.out.println(this + " - stop requested.");
+        _doSpin = false;
+    }
+
+}