8005418: JSR 292: virtual dispatch bug in 292 impl

Reviewed-by: jrose, kvn

src/share/vm/opto/callGenerator.cpp

@@ -717,6 +717,7 @@ CallGenerator* CallGenerator::for_method_handle_call(JVMState* jvms, ciMethod* c
       (input_not_const || !C->inlining_incrementally() || C->over_inlining_cutoff())) {
     return CallGenerator::for_mh_late_inline(caller, callee, input_not_const);
   } else {
+    // Out-of-line call.
     return CallGenerator::for_direct_call(callee);
   }
 }
@@ -739,7 +740,7 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
         guarantee(!target->is_method_handle_intrinsic(), "should not happen");  // XXX remove
         const int vtable_index = Method::invalid_vtable_index;
         CallGenerator* cg = C->call_generator(target, vtable_index, false, jvms, true, PROB_ALWAYS, true, true);
-        assert (!cg->is_late_inline() || cg->is_mh_late_inline(), "no late inline here");
+        assert(!cg->is_late_inline() || cg->is_mh_late_inline(), "no late inline here");
         if (cg != NULL && cg->is_inline())
           return cg;
       }
@@ -787,10 +788,25 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
             }
           }
         }
-        const int vtable_index = Method::invalid_vtable_index;
-        const bool call_is_virtual = target->is_abstract();  // FIXME workaround
-        CallGenerator* cg = C->call_generator(target, vtable_index, call_is_virtual, jvms, true, PROB_ALWAYS, true, true);
-        assert (!cg->is_late_inline() || cg->is_mh_late_inline(), "no late inline here");
+
+        // Try to get the most accurate receiver type
+        const bool is_virtual              = (iid == vmIntrinsics::_linkToVirtual);
+        const bool is_virtual_or_interface = (is_virtual || iid == vmIntrinsics::_linkToInterface);
+        int  vtable_index       = Method::invalid_vtable_index;
+        bool call_does_dispatch = false;
+
+        if (is_virtual_or_interface) {
+          ciInstanceKlass* klass = target->holder();
+          Node*             receiver_node = kit.argument(0);
+          const TypeOopPtr* receiver_type = gvn.type(receiver_node)->isa_oopptr();
+          // call_does_dispatch and vtable_index are out-parameters.  They might be changed.
+          target = C->optimize_virtual_call(caller, jvms->bci(), klass, target, receiver_type,
+                                            is_virtual,
+                                            call_does_dispatch, vtable_index);  // out-parameters
+        }
+
+        CallGenerator* cg = C->call_generator(target, vtable_index, call_does_dispatch, jvms, true, PROB_ALWAYS, true, true);
+        assert(!cg->is_late_inline() || cg->is_mh_late_inline(), "no late inline here");
         if (cg != NULL && cg->is_inline())
           return cg;
       }

src/share/vm/opto/compile.hpp

@@ -72,6 +72,7 @@ class SafePointNode;
 class JVMState;
 class TypeData;
 class TypePtr;
+class TypeOopPtr;
 class TypeFunc;
 class Unique_Node_List;
 class nmethod;
@@ -740,9 +741,17 @@ class Compile : public Phase {
 
   // Decide how to build a call.
   // The profile factor is a discount to apply to this site's interp. profile.
-  CallGenerator*    call_generator(ciMethod* call_method, int vtable_index, bool call_is_virtual, JVMState* jvms, bool allow_inline, float profile_factor, bool allow_intrinsics = true, bool delayed_forbidden = false);
+  CallGenerator*    call_generator(ciMethod* call_method, int vtable_index, bool call_does_dispatch, JVMState* jvms, bool allow_inline, float profile_factor, bool allow_intrinsics = true, bool delayed_forbidden = false);
   bool should_delay_inlining(ciMethod* call_method, JVMState* jvms);
 
+  // Helper functions to identify inlining potential at call-site
+  ciMethod* optimize_virtual_call(ciMethod* caller, int bci, ciInstanceKlass* klass,
+                                  ciMethod* callee, const TypeOopPtr* receiver_type,
+                                  bool is_virtual,
+                                  bool &call_does_dispatch, int &vtable_index);
+  ciMethod* optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
+                              ciMethod* callee, const TypeOopPtr* receiver_type);
+
   // Report if there were too many traps at a current method and bci.
   // Report if a trap was recorded, and/or PerMethodTrapLimit was exceeded.
   // If there is no MDO at all, report no trap unless told to assume it.

src/share/vm/opto/doCall.cpp

@@ -61,7 +61,7 @@ void trace_type_profile(Compile* C, ciMethod *method, int depth, int bci, ciMeth
   }
 }
 
-CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool call_is_virtual,
+CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool call_does_dispatch,
                                        JVMState* jvms, bool allow_inline,
                                        float prof_factor, bool allow_intrinsics, bool delayed_forbidden) {
   ciMethod*       caller   = jvms->method();
@@ -82,7 +82,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
   // See how many times this site has been invoked.
   int site_count = profile.count();
   int receiver_count = -1;
-  if (call_is_virtual && UseTypeProfile && profile.has_receiver(0)) {
+  if (call_does_dispatch && UseTypeProfile && profile.has_receiver(0)) {
     // Receivers in the profile structure are ordered by call counts
     // so that the most called (major) receiver is profile.receiver(0).
     receiver_count = profile.receiver_count(0);
@@ -94,7 +94,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
     int r2id = (rid != -1 && profile.has_receiver(1))? log->identify(profile.receiver(1)):-1;
     log->begin_elem("call method='%d' count='%d' prof_factor='%g'",
                     log->identify(callee), site_count, prof_factor);
-    if (call_is_virtual)  log->print(" virtual='1'");
+    if (call_does_dispatch)  log->print(" virtual='1'");
     if (allow_inline)     log->print(" inline='1'");
     if (receiver_count >= 0) {
       log->print(" receiver='%d' receiver_count='%d'", rid, receiver_count);
@@ -111,12 +111,12 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
   // We do this before the strict f.p. check below because the
   // intrinsics handle strict f.p. correctly.
   if (allow_inline && allow_intrinsics) {
-    CallGenerator* cg = find_intrinsic(callee, call_is_virtual);
+    CallGenerator* cg = find_intrinsic(callee, call_does_dispatch);
     if (cg != NULL) {
       if (cg->is_predicted()) {
         // Code without intrinsic but, hopefully, inlined.
         CallGenerator* inline_cg = this->call_generator(callee,
-              vtable_index, call_is_virtual, jvms, allow_inline, prof_factor, false);
+              vtable_index, call_does_dispatch, jvms, allow_inline, prof_factor, false);
         if (inline_cg != NULL) {
           cg = CallGenerator::for_predicted_intrinsic(cg, inline_cg);
         }
@@ -131,7 +131,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
   // have bytecodes and so normal inlining fails.
   if (callee->is_method_handle_intrinsic()) {
     CallGenerator* cg = CallGenerator::for_method_handle_call(jvms, caller, callee, delayed_forbidden);
-    assert (cg == NULL || !delayed_forbidden || !cg->is_late_inline() || cg->is_mh_late_inline(), "unexpected CallGenerator");
+    assert(cg == NULL || !delayed_forbidden || !cg->is_late_inline() || cg->is_mh_late_inline(), "unexpected CallGenerator");
     return cg;
   }
 
@@ -149,7 +149,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
     float expected_uses = past_uses;
 
     // Try inlining a bytecoded method:
-    if (!call_is_virtual) {
+    if (!call_does_dispatch) {
       InlineTree* ilt;
       if (UseOldInlining) {
         ilt = InlineTree::find_subtree_from_root(this->ilt(), jvms->caller(), jvms->method());
@@ -188,14 +188,14 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
         } else if (require_inline || !InlineWarmCalls) {
           return cg;
         } else {
-          CallGenerator* cold_cg = call_generator(callee, vtable_index, call_is_virtual, jvms, false, prof_factor);
+          CallGenerator* cold_cg = call_generator(callee, vtable_index, call_does_dispatch, jvms, false, prof_factor);
           return CallGenerator::for_warm_call(ci, cold_cg, cg);
         }
       }
     }
 
     // Try using the type profile.
-    if (call_is_virtual && site_count > 0 && receiver_count > 0) {
+    if (call_does_dispatch && site_count > 0 && receiver_count > 0) {
       // The major receiver's count >= TypeProfileMajorReceiverPercent of site_count.
       bool have_major_receiver = (100.*profile.receiver_prob(0) >= (float)TypeProfileMajorReceiverPercent);
       ciMethod* receiver_method = NULL;
@@ -209,7 +209,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
       if (receiver_method != NULL) {
         // The single majority receiver sufficiently outweighs the minority.
         CallGenerator* hit_cg = this->call_generator(receiver_method,
-              vtable_index, !call_is_virtual, jvms, allow_inline, prof_factor);
+              vtable_index, !call_does_dispatch, jvms, allow_inline, prof_factor);
         if (hit_cg != NULL) {
           // Look up second receiver.
           CallGenerator* next_hit_cg = NULL;
@@ -219,7 +219,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
                                                                profile.receiver(1));
             if (next_receiver_method != NULL) {
               next_hit_cg = this->call_generator(next_receiver_method,
-                                  vtable_index, !call_is_virtual, jvms,
+                                  vtable_index, !call_does_dispatch, jvms,
                                   allow_inline, prof_factor);
               if (next_hit_cg != NULL && !next_hit_cg->is_inline() &&
                   have_major_receiver && UseOnlyInlinedBimorphic) {
@@ -265,7 +265,7 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
 
   // There was no special inlining tactic, or it bailed out.
   // Use a more generic tactic, like a simple call.
-  if (call_is_virtual) {
+  if (call_does_dispatch) {
     return CallGenerator::for_virtual_call(callee, vtable_index);
   } else {
     // Class Hierarchy Analysis or Type Profile reveals a unique target,
@@ -397,6 +397,7 @@ void Parse::do_call() {
   // orig_callee is the resolved callee which's signature includes the
   // appendix argument.
   const int nargs = orig_callee->arg_size();
+  const bool is_signature_polymorphic = MethodHandles::is_signature_polymorphic(orig_callee->intrinsic_id());
 
   // Push appendix argument (MethodType, CallSite, etc.), if one.
   if (iter().has_appendix()) {
@@ -413,25 +414,18 @@ void Parse::do_call() {
   // Then we may introduce a run-time check and inline on the path where it succeeds.
   // The other path may uncommon_trap, check for another receiver, or do a v-call.
 
-  // Choose call strategy.
-  bool call_is_virtual = is_virtual_or_interface;
-  int vtable_index = Method::invalid_vtable_index;
-  ciMethod* callee = orig_callee;
-
   // Try to get the most accurate receiver type
+  ciMethod* callee             = orig_callee;
+  int       vtable_index       = Method::invalid_vtable_index;
+  bool      call_does_dispatch = false;
+
   if (is_virtual_or_interface) {
     Node*             receiver_node = stack(sp() - nargs);
     const TypeOopPtr* receiver_type = _gvn.type(receiver_node)->isa_oopptr();
-    ciMethod* optimized_virtual_method = optimize_inlining(method(), bci(), klass, orig_callee, receiver_type);
-
-    // Have the call been sufficiently improved such that it is no longer a virtual?
-    if (optimized_virtual_method != NULL) {
-      callee          = optimized_virtual_method;
-      call_is_virtual = false;
-    } else if (!UseInlineCaches && is_virtual && callee->is_loaded()) {
-      // We can make a vtable call at this site
-      vtable_index = callee->resolve_vtable_index(method()->holder(), klass);
-    }
+    // call_does_dispatch and vtable_index are out-parameters.  They might be changed.
+    callee = C->optimize_virtual_call(method(), bci(), klass, orig_callee, receiver_type,
+                                      is_virtual,
+                                      call_does_dispatch, vtable_index);  // out-parameters
   }
 
   // Note:  It's OK to try to inline a virtual call.
@@ -447,7 +441,7 @@ void Parse::do_call() {
   // Decide call tactic.
   // This call checks with CHA, the interpreter profile, intrinsics table, etc.
   // It decides whether inlining is desirable or not.
-  CallGenerator* cg = C->call_generator(callee, vtable_index, call_is_virtual, jvms, try_inline, prof_factor());
+  CallGenerator* cg = C->call_generator(callee, vtable_index, call_does_dispatch, jvms, try_inline, prof_factor());
 
   // NOTE:  Don't use orig_callee and callee after this point!  Use cg->method() instead.
   orig_callee = callee = NULL;
@@ -487,7 +481,7 @@ void Parse::do_call() {
     // the call site, perhaps because it did not match a pattern the
     // intrinsic was expecting to optimize. Should always be possible to
     // get a normal java call that may inline in that case
-    cg = C->call_generator(cg->method(), vtable_index, call_is_virtual, jvms, try_inline, prof_factor(), /* allow_intrinsics= */ false);
+    cg = C->call_generator(cg->method(), vtable_index, call_does_dispatch, jvms, try_inline, prof_factor(), /* allow_intrinsics= */ false);
     if ((new_jvms = cg->generate(jvms)) == NULL) {
       guarantee(failing(), "call failed to generate:  calls should work");
       return;
@@ -522,55 +516,44 @@ void Parse::do_call() {
     round_double_result(cg->method());
 
     ciType* rtype = cg->method()->return_type();
-    if (Bytecodes::has_optional_appendix(iter().cur_bc_raw())) {
+    ciType* ctype = declared_signature->return_type();
+
+    if (Bytecodes::has_optional_appendix(iter().cur_bc_raw()) || is_signature_polymorphic) {
       // Be careful here with return types.
-      ciType* ctype = declared_signature->return_type();
       if (ctype != rtype) {
         BasicType rt = rtype->basic_type();
         BasicType ct = ctype->basic_type();
-        Node* retnode = peek();
         if (ct == T_VOID) {
           // It's OK for a method  to return a value that is discarded.
           // The discarding does not require any special action from the caller.
           // The Java code knows this, at VerifyType.isNullConversion.
           pop_node(rt);  // whatever it was, pop it
-          retnode = top();
         } else if (rt == T_INT || is_subword_type(rt)) {
-          // FIXME: This logic should be factored out.
-          if (ct == T_BOOLEAN) {
-            retnode = _gvn.transform( new (C) AndINode(retnode, intcon(0x1)) );
-          } else if (ct == T_CHAR) {
-            retnode = _gvn.transform( new (C) AndINode(retnode, intcon(0xFFFF)) );
-          } else if (ct == T_BYTE) {
-            retnode = _gvn.transform( new (C) LShiftINode(retnode, intcon(24)) );
-            retnode = _gvn.transform( new (C) RShiftINode(retnode, intcon(24)) );
-          } else if (ct == T_SHORT) {
-            retnode = _gvn.transform( new (C) LShiftINode(retnode, intcon(16)) );
-            retnode = _gvn.transform( new (C) RShiftINode(retnode, intcon(16)) );
-          } else {
-            assert(ct == T_INT, err_msg_res("rt=%s, ct=%s", type2name(rt), type2name(ct)));
-          }
+          // Nothing.  These cases are handled in lambda form bytecode.
+          assert(ct == T_INT || is_subword_type(ct), err_msg_res("must match: rt=%s, ct=%s", type2name(rt), type2name(ct)));
         } else if (rt == T_OBJECT || rt == T_ARRAY) {
           assert(ct == T_OBJECT || ct == T_ARRAY, err_msg_res("rt=%s, ct=%s", type2name(rt), type2name(ct)));
           if (ctype->is_loaded()) {
             const TypeOopPtr* arg_type = TypeOopPtr::make_from_klass(rtype->as_klass());
             const Type*       sig_type = TypeOopPtr::make_from_klass(ctype->as_klass());
             if (arg_type != NULL && !arg_type->higher_equal(sig_type)) {
+              Node* retnode = pop();
               Node* cast_obj = _gvn.transform(new (C) CheckCastPPNode(control(), retnode, sig_type));
-              pop();
               push(cast_obj);
             }
           }
         } else {
-          assert(ct == rt, err_msg("unexpected mismatch rt=%d, ct=%d", rt, ct));
+          assert(rt == ct, err_msg_res("unexpected mismatch: rt=%s, ct=%s", type2name(rt), type2name(ct)));
           // push a zero; it's better than getting an oop/int mismatch
-          retnode = pop_node(rt);
-          retnode = zerocon(ct);
+          pop_node(rt);
+          Node* retnode = zerocon(ct);
           push_node(ct, retnode);
         }
         // Now that the value is well-behaved, continue with the call-site type.
         rtype = ctype;
       }
+    } else {
+      assert(rtype == ctype, "mismatched return types");  // symbolic resolution enforces this
     }
 
     // If the return type of the method is not loaded, assert that the
@@ -888,17 +871,39 @@ void Parse::count_compiled_calls(bool at_method_entry, bool is_inline) {
 #endif //PRODUCT
 
 
+ciMethod* Compile::optimize_virtual_call(ciMethod* caller, int bci, ciInstanceKlass* klass,
+                                         ciMethod* callee, const TypeOopPtr* receiver_type,
+                                         bool is_virtual,
+                                         bool& call_does_dispatch, int& vtable_index) {
+  // Set default values for out-parameters.
+  call_does_dispatch = true;
+  vtable_index       = Method::invalid_vtable_index;
+
+  // Choose call strategy.
+  ciMethod* optimized_virtual_method = optimize_inlining(caller, bci, klass, callee, receiver_type);
+
+  // Have the call been sufficiently improved such that it is no longer a virtual?
+  if (optimized_virtual_method != NULL) {
+    callee             = optimized_virtual_method;
+    call_does_dispatch = false;
+  } else if (!UseInlineCaches && is_virtual && callee->is_loaded()) {
+    // We can make a vtable call at this site
+    vtable_index = callee->resolve_vtable_index(caller->holder(), klass);
+  }
+  return callee;
+}
+
 // Identify possible target method and inlining style
-ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
-                                   ciMethod *dest_method, const TypeOopPtr* receiver_type) {
+ciMethod* Compile::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
+                                     ciMethod* callee, const TypeOopPtr* receiver_type) {
   // only use for virtual or interface calls
 
   // If it is obviously final, do not bother to call find_monomorphic_target,
   // because the class hierarchy checks are not needed, and may fail due to
   // incompletely loaded classes.  Since we do our own class loading checks
   // in this module, we may confidently bind to any method.
-  if (dest_method->can_be_statically_bound()) {
-    return dest_method;
+  if (callee->can_be_statically_bound()) {
+    return callee;
   }
 
   // Attempt to improve the receiver
@@ -907,8 +912,8 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
   if (receiver_type != NULL) {
     // Array methods are all inherited from Object, and are monomorphic.
     if (receiver_type->isa_aryptr() &&
-        dest_method->holder() == env()->Object_klass()) {
-      return dest_method;
+        callee->holder() == env()->Object_klass()) {
+      return callee;
     }
 
     // All other interesting cases are instance klasses.
@@ -928,7 +933,7 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
   }
 
   ciInstanceKlass*   calling_klass = caller->holder();
-  ciMethod* cha_monomorphic_target = dest_method->find_monomorphic_target(calling_klass, klass, actual_receiver);
+  ciMethod* cha_monomorphic_target = callee->find_monomorphic_target(calling_klass, klass, actual_receiver);
   if (cha_monomorphic_target != NULL) {
     assert(!cha_monomorphic_target->is_abstract(), "");
     // Look at the method-receiver type.  Does it add "too much information"?
@@ -946,10 +951,10 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
         cha_monomorphic_target->print();
         tty->cr();
       }
-      if (C->log() != NULL) {
-        C->log()->elem("missed_CHA_opportunity klass='%d' method='%d'",
-                       C->log()->identify(klass),
-                       C->log()->identify(cha_monomorphic_target));
+      if (log() != NULL) {
+        log()->elem("missed_CHA_opportunity klass='%d' method='%d'",
+                       log()->identify(klass),
+                       log()->identify(cha_monomorphic_target));
       }
       cha_monomorphic_target = NULL;
     }
@@ -961,7 +966,7 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
     // by dynamic class loading.  Be sure to test the "static" receiver
     // dest_method here, as opposed to the actual receiver, which may
     // falsely lead us to believe that the receiver is final or private.
-    C->dependencies()->assert_unique_concrete_method(actual_receiver, cha_monomorphic_target);
+    dependencies()->assert_unique_concrete_method(actual_receiver, cha_monomorphic_target);
     return cha_monomorphic_target;
   }
 
@@ -970,7 +975,7 @@ ciMethod* Parse::optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* k
   if (actual_receiver_is_exact) {
     // In case of evolution, there is a dependence on every inlined method, since each
     // such method can be changed when its class is redefined.
-    ciMethod* exact_method = dest_method->resolve_invoke(calling_klass, actual_receiver);
+    ciMethod* exact_method = callee->resolve_invoke(calling_klass, actual_receiver);
     if (exact_method != NULL) {
 #ifndef PRODUCT
       if (PrintOpto) {

src/share/vm/opto/parse.hpp

@@ -469,10 +469,6 @@ class Parse : public GraphKit {
   // Helper function to uncommon-trap or bailout for non-compilable call-sites
   bool can_not_compile_call_site(ciMethod *dest_method, ciInstanceKlass *klass);
 
-  // Helper function to identify inlining potential at call-site
-  ciMethod* optimize_inlining(ciMethod* caller, int bci, ciInstanceKlass* klass,
-                              ciMethod *dest_method, const TypeOopPtr* receiver_type);
-
   // Helper function to setup for type-profile based inlining
   bool prepare_type_profile_inline(ciInstanceKlass* prof_klass, ciMethod* prof_method);
 

src/share/vm/opto/parse1.cpp

@@ -1404,7 +1404,8 @@ void Parse::do_one_block() {
 
     do_one_bytecode();
 
-    assert(!have_se || stopped() || failing() || (sp() - pre_bc_sp) == depth, "correct depth prediction");
+    assert(!have_se || stopped() || failing() || (sp() - pre_bc_sp) == depth,
+           err_msg_res("incorrect depth prediction: sp=%d, pre_bc_sp=%d, depth=%d", sp(), pre_bc_sp, depth));
 
     do_exceptions();