8027631: "unexpected profiling mismatch" error with new type profiling
authorroland
Wed, 13 Nov 2013 09:45:58 +0100
changeset 21579 c396d68f3e48
parent 21576 e7cac8551ec2
child 21580 488e43800f51
8027631: "unexpected profiling mismatch" error with new type profiling Summary: inlined method handle calls can call methods with different signatures Reviewed-by: kvn, iveresov
hotspot/src/share/vm/c1/c1_LIRGenerator.cpp
hotspot/src/share/vm/c1/c1_LIRGenerator.hpp
hotspot/test/compiler/profiling/TestUnexpectedProfilingMismatch.java
--- a/hotspot/src/share/vm/c1/c1_LIRGenerator.cpp	Tue Nov 12 13:58:37 2013 +0100
+++ b/hotspot/src/share/vm/c1/c1_LIRGenerator.cpp	Wed Nov 13 09:45:58 2013 +0100
@@ -2574,8 +2574,25 @@
   __ jump(x->default_sux());
 }
 
-
-ciKlass* LIRGenerator::profile_arg_type(ciMethodData* md, int md_base_offset, int md_offset, intptr_t profiled_k, Value arg, LIR_Opr& mdp, bool not_null, ciKlass* signature_k) {
+/**
+ * Emit profiling code if needed for arguments, parameters, return value types
+ *
+ * @param md                    MDO the code will update at runtime
+ * @param md_base_offset        common offset in the MDO for this profile and subsequent ones
+ * @param md_offset             offset in the MDO (on top of md_base_offset) for this profile
+ * @param profiled_k            current profile
+ * @param obj                   IR node for the object to be profiled
+ * @param mdp                   register to hold the pointer inside the MDO (md + md_base_offset).
+ *                              Set once we find an update to make and use for next ones.
+ * @param not_null              true if we know obj cannot be null
+ * @param signature_at_call_k   signature at call for obj
+ * @param callee_signature_k    signature of callee for obj
+ *                              at call and callee signatures differ at method handle call
+ * @return                      the only klass we know will ever be seen at this profile point
+ */
+ciKlass* LIRGenerator::profile_type(ciMethodData* md, int md_base_offset, int md_offset, intptr_t profiled_k,
+                                    Value obj, LIR_Opr& mdp, bool not_null, ciKlass* signature_at_call_k,
+                                    ciKlass* callee_signature_k) {
   ciKlass* result = NULL;
   bool do_null = !not_null && !TypeEntries::was_null_seen(profiled_k);
   bool do_update = !TypeEntries::is_type_unknown(profiled_k);
@@ -2590,9 +2607,9 @@
   if (do_update) {
     // try to find exact type, using CHA if possible, so that loading
     // the klass from the object can be avoided
-    ciType* type = arg->exact_type();
+    ciType* type = obj->exact_type();
     if (type == NULL) {
-      type = arg->declared_type();
+      type = obj->declared_type();
       type = comp->cha_exact_type(type);
     }
     assert(type == NULL || type->is_klass(), "type should be class");
@@ -2608,23 +2625,33 @@
   ciKlass* exact_signature_k = NULL;
   if (do_update) {
     // Is the type from the signature exact (the only one possible)?
-    exact_signature_k = signature_k->exact_klass();
+    exact_signature_k = signature_at_call_k->exact_klass();
     if (exact_signature_k == NULL) {
-      exact_signature_k = comp->cha_exact_type(signature_k);
+      exact_signature_k = comp->cha_exact_type(signature_at_call_k);
     } else {
       result = exact_signature_k;
-      do_update = false;
       // Known statically. No need to emit any code: prevent
       // LIR_Assembler::emit_profile_type() from emitting useless code
       profiled_k = ciTypeEntries::with_status(result, profiled_k);
     }
     if (exact_signature_k != NULL && exact_klass != exact_signature_k) {
-      assert(exact_klass == NULL, "arg and signature disagree?");
+      assert(exact_klass == NULL, "obj and signature disagree?");
       // sometimes the type of the signature is better than the best type
       // the compiler has
       exact_klass = exact_signature_k;
-      do_update = exact_klass == NULL || ciTypeEntries::valid_ciklass(profiled_k) != exact_klass;
     }
+    if (callee_signature_k != NULL &&
+        callee_signature_k != signature_at_call_k) {
+      ciKlass* improved_klass = callee_signature_k->exact_klass();
+      if (improved_klass == NULL) {
+        improved_klass = comp->cha_exact_type(callee_signature_k);
+      }
+      if (improved_klass != NULL && exact_klass != improved_klass) {
+        assert(exact_klass == NULL, "obj and signature disagree?");
+        exact_klass = exact_signature_k;
+      }
+    }
+    do_update = exact_klass == NULL || ciTypeEntries::valid_ciklass(profiled_k) != exact_klass;
   }
 
   if (!do_null && !do_update) {
@@ -2640,7 +2667,7 @@
       __ leal(LIR_OprFact::address(base_type_address), mdp);
     }
   }
-  LIRItem value(arg, this);
+  LIRItem value(obj, this);
   value.load_item();
   __ profile_type(new LIR_Address(mdp, md_offset, T_METADATA),
                   value.result(), exact_klass, profiled_k, new_pointer_register(), not_null, exact_signature_k != NULL);
@@ -2665,9 +2692,9 @@
         if (t == T_OBJECT || t == T_ARRAY) {
           intptr_t profiled_k = parameters->type(j);
           Local* local = x->state()->local_at(java_index)->as_Local();
-          ciKlass* exact = profile_arg_type(md, md->byte_offset_of_slot(parameters_type_data, ParametersTypeData::type_offset(0)),
-                                            in_bytes(ParametersTypeData::type_offset(j)) - in_bytes(ParametersTypeData::type_offset(0)),
-                                            profiled_k, local, mdp, false, local->declared_type()->as_klass());
+          ciKlass* exact = profile_type(md, md->byte_offset_of_slot(parameters_type_data, ParametersTypeData::type_offset(0)),
+                                        in_bytes(ParametersTypeData::type_offset(j)) - in_bytes(ParametersTypeData::type_offset(0)),
+                                        profiled_k, local, mdp, false, local->declared_type()->as_klass(), NULL);
           // If the profile is known statically set it once for all and do not emit any code
           if (exact != NULL) {
             md->set_parameter_type(j, exact);
@@ -3129,19 +3156,28 @@
       Bytecodes::Code bc = x->method()->java_code_at_bci(bci);
       int start = 0;
       int stop = data->is_CallTypeData() ? ((ciCallTypeData*)data)->number_of_arguments() : ((ciVirtualCallTypeData*)data)->number_of_arguments();
-      if (x->nb_profiled_args() < stop) {
-        // if called through method handle invoke, some arguments may have been popped
-        stop = x->nb_profiled_args();
+      if (x->inlined() && x->callee()->is_static() && Bytecodes::has_receiver(bc)) {
+        // first argument is not profiled at call (method handle invoke)
+        assert(x->method()->raw_code_at_bci(bci) == Bytecodes::_invokehandle, "invokehandle expected");
+        start = 1;
       }
-      ciSignature* sig = x->callee()->signature();
+      ciSignature* callee_signature = x->callee()->signature();
       // method handle call to virtual method
       bool has_receiver = x->inlined() && !x->callee()->is_static() && !Bytecodes::has_receiver(bc);
-      ciSignatureStream sig_stream(sig, has_receiver ? x->callee()->holder() : NULL);
-      for (int i = 0; i < stop; i++) {
+      ciSignatureStream callee_signature_stream(callee_signature, has_receiver ? x->callee()->holder() : NULL);
+
+      bool ignored_will_link;
+      ciSignature* signature_at_call = NULL;
+      x->method()->get_method_at_bci(bci, ignored_will_link, &signature_at_call);
+      ciSignatureStream signature_at_call_stream(signature_at_call);
+
+      // if called through method handle invoke, some arguments may have been popped
+      for (int i = 0; i < stop && i+start < x->nb_profiled_args(); i++) {
         int off = in_bytes(TypeEntriesAtCall::argument_type_offset(i)) - in_bytes(TypeEntriesAtCall::args_data_offset());
-        ciKlass* exact = profile_arg_type(md, base_offset, off,
-                                          args->type(i), x->profiled_arg_at(i+start), mdp,
-                                          !x->arg_needs_null_check(i+start), sig_stream.next_klass());
+        ciKlass* exact = profile_type(md, base_offset, off,
+                                      args->type(i), x->profiled_arg_at(i+start), mdp,
+                                      !x->arg_needs_null_check(i+start),
+                                      signature_at_call_stream.next_klass(), callee_signature_stream.next_klass());
         if (exact != NULL) {
           md->set_argument_type(bci, i, exact);
         }
@@ -3176,8 +3212,8 @@
         int bci = x->bci_of_invoke();
         Bytecodes::Code bc = x->method()->java_code_at_bci(bci);
         // The first parameter is the receiver so that's what we start
-        // with if it exists. On exception if method handle call to
-        // virtual method has receiver in the args list
+        // with if it exists. One exception is method handle call to
+        // virtual method: the receiver is in the args list
         if (arg == NULL || !Bytecodes::has_receiver(bc)) {
           i = 1;
           arg = x->profiled_arg_at(0);
@@ -3186,9 +3222,9 @@
         int k = 0; // to iterate on the profile data
         for (;;) {
           intptr_t profiled_k = parameters->type(k);
-          ciKlass* exact = profile_arg_type(md, md->byte_offset_of_slot(parameters_type_data, ParametersTypeData::type_offset(0)),
-                                            in_bytes(ParametersTypeData::type_offset(k)) - in_bytes(ParametersTypeData::type_offset(0)),
-                                            profiled_k, arg, mdp, not_null, sig_stream.next_klass());
+          ciKlass* exact = profile_type(md, md->byte_offset_of_slot(parameters_type_data, ParametersTypeData::type_offset(0)),
+                                        in_bytes(ParametersTypeData::type_offset(k)) - in_bytes(ParametersTypeData::type_offset(0)),
+                                        profiled_k, arg, mdp, not_null, sig_stream.next_klass(), NULL);
           // If the profile is known statically set it once for all and do not emit any code
           if (exact != NULL) {
             md->set_parameter_type(k, exact);
@@ -3247,9 +3283,16 @@
   assert(data->is_CallTypeData() || data->is_VirtualCallTypeData(), "wrong profile data type");
   ciReturnTypeEntry* ret = data->is_CallTypeData() ? ((ciCallTypeData*)data)->ret() : ((ciVirtualCallTypeData*)data)->ret();
   LIR_Opr mdp = LIR_OprFact::illegalOpr;
-  ciKlass* exact = profile_arg_type(md, 0, md->byte_offset_of_slot(data, ret->type_offset()),
-                                    ret->type(), x->ret(), mdp,
-                                    !x->needs_null_check(), x->callee()->signature()->return_type()->as_klass());
+
+  bool ignored_will_link;
+  ciSignature* signature_at_call = NULL;
+  x->method()->get_method_at_bci(bci, ignored_will_link, &signature_at_call);
+
+  ciKlass* exact = profile_type(md, 0, md->byte_offset_of_slot(data, ret->type_offset()),
+                                ret->type(), x->ret(), mdp,
+                                !x->needs_null_check(),
+                                signature_at_call->return_type()->as_klass(),
+                                x->callee()->signature()->return_type()->as_klass());
   if (exact != NULL) {
     md->set_return_type(bci, exact);
   }
--- a/hotspot/src/share/vm/c1/c1_LIRGenerator.hpp	Tue Nov 12 13:58:37 2013 +0100
+++ b/hotspot/src/share/vm/c1/c1_LIRGenerator.hpp	Wed Nov 13 09:45:58 2013 +0100
@@ -434,7 +434,9 @@
   void do_ThreadIDIntrinsic(Intrinsic* x);
   void do_ClassIDIntrinsic(Intrinsic* x);
 #endif
-  ciKlass* profile_arg_type(ciMethodData* md, int md_first_offset, int md_offset, intptr_t profiled_k, Value arg, LIR_Opr& mdp, bool not_null, ciKlass* signature_k);
+  ciKlass* profile_type(ciMethodData* md, int md_first_offset, int md_offset, intptr_t profiled_k,
+                        Value arg, LIR_Opr& mdp, bool not_null, ciKlass* signature_at_call_k,
+                        ciKlass* callee_signature_k);
   void profile_arguments(ProfileCall* x);
   void profile_parameters(Base* x);
   void profile_parameters_at_call(ProfileCall* x);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/profiling/TestUnexpectedProfilingMismatch.java	Wed Nov 13 09:45:58 2013 +0100
@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2013, 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
+ * 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 8027631
+ * @summary profiling of arguments at calls cannot rely on signature of callee for types
+ * @run main/othervm -XX:-BackgroundCompilation -XX:TieredStopAtLevel=3 -XX:TypeProfileLevel=111 -XX:Tier3InvocationThreshold=200 -XX:Tier0InvokeNotifyFreqLog=7 TestUnexpectedProfilingMismatch
+ *
+ */
+
+import java.lang.invoke.*;
+
+public class TestUnexpectedProfilingMismatch {
+
+    static class A {
+    }
+
+    static class B {
+    }
+
+    static void mA(A a) {
+    }
+
+    static void mB(B b) {
+    }
+
+    static final MethodHandle mhA;
+    static final MethodHandle mhB;
+    static {
+        MethodHandles.Lookup lookup = MethodHandles.lookup();
+        MethodType mt = MethodType.methodType(void.class, A.class);
+        MethodHandle res = null;
+        try {
+            res = lookup.findStatic(TestUnexpectedProfilingMismatch.class, "mA", mt);
+        } catch(NoSuchMethodException ex) {
+        } catch(IllegalAccessException ex) {
+        }
+        mhA = res;
+        mt = MethodType.methodType(void.class, B.class);
+        try {
+            res = lookup.findStatic(TestUnexpectedProfilingMismatch.class, "mB", mt);
+        } catch(NoSuchMethodException ex) {
+        } catch(IllegalAccessException ex) {
+        }
+        mhB = res;
+    }
+
+    void m1(A a, boolean doit) throws Throwable {
+        if (doit) {
+            mhA.invoke(a);
+        }
+    }
+
+    void m2(B b) throws Throwable {
+        mhB.invoke(b);
+    }
+
+    static public void main(String[] args) {
+        TestUnexpectedProfilingMismatch tih = new TestUnexpectedProfilingMismatch();
+        A a = new A();
+        B b = new B();
+        try {
+            for (int i = 0; i < 256 - 1; i++) {
+                tih.m1(a, true);
+            }
+            // Will trigger the compilation but will also run once
+            // more interpreted with a non null MDO which it will
+            // update. Make it skip the body of the method.
+            tih.m1(a, false);
+            // Compile this one as well and do the profiling
+            for (int i = 0; i < 256; i++) {
+                tih.m2(b);
+            }
+            // Will run and see a conflict
+            tih.m1(a, true);
+        } catch(Throwable ex) {
+            ex.printStackTrace();
+        }
+        System.out.println("TEST PASSED");
+    }
+}