changeset 55115:2d8d0287bc22 lworld

8223229: [lworld] C1 crashes when calling final virtual methods with value arguments Reviewed-by: thartmann
author iklam
date Fri, 03 May 2019 16:37:19 -0700
parents c7ecf40f8a60
children ac891b47cc41
files src/hotspot/share/code/compiledIC.cpp src/hotspot/share/code/compiledMethod.hpp src/hotspot/share/code/nmethod.hpp src/hotspot/share/runtime/sharedRuntime.cpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConventionC1.java
diffstat 5 files changed, 21 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/code/compiledIC.cpp	Thu May 02 16:10:02 2019 -0700
+++ b/src/hotspot/share/code/compiledIC.cpp	Fri May 03 16:37:19 2019 -0700
@@ -536,8 +536,9 @@
     //     it looks vanilla but is optimized. Code in is_call_to_interpreted
     //     is aware of this and weakens its asserts.
     if (is_optimized) {
-      entry      = method_code->verified_entry_point();
+      entry      = caller_is_c1 ? method_code->verified_value_entry_point() : method_code->verified_entry_point();
     } else {
+    //assert(!(caller_is_c1 && method->has_scalarized_args()), "FIXME - what to do with c1 caller??");
       entry      = method_code->entry_point();
     }
   }
@@ -559,6 +560,7 @@
       // Use icholder entry
       assert(method_code == NULL || method_code->is_compiled(), "must be compiled");
       CompiledICHolder* holder = new CompiledICHolder(method(), receiver_klass);
+    //assert(!(caller_is_c1 && method->has_scalarized_args()), "FIXME - what to do with c1 caller??");
       info.set_icholder_entry(method()->get_c2i_unverified_entry(), holder);
     }
   }
@@ -670,7 +672,7 @@
       info._to_aot = false;
     }
     info._to_interpreter = false;
-    if (caller_nm->is_c1()) {
+    if (caller_nm->is_compiled_by_c1()) {
       info._entry = m_code->verified_value_entry_point();
     } else {
       info._entry = m_code->verified_entry_point();
@@ -681,7 +683,7 @@
     assert(!m->is_method_handle_intrinsic(), "Compiled code should never call interpreter MH intrinsics");
     info._to_interpreter = true;
 
-    if (caller_nm->is_c1()) {
+    if (caller_nm->is_compiled_by_c1()) {
       // C1 -> interp: values passed as oops
       info._entry = m()->get_c2i_value_entry();
     } else {
--- a/src/hotspot/share/code/compiledMethod.hpp	Thu May 02 16:10:02 2019 -0700
+++ b/src/hotspot/share/code/compiledMethod.hpp	Fri May 03 16:37:19 2019 -0700
@@ -225,7 +225,6 @@
 
   virtual bool  is_in_use() const = 0;
   virtual int   comp_level() const = 0;
-  virtual bool  is_c1() const { return false; }
   virtual int   compile_id() const = 0;
 
   virtual address verified_entry_point() const = 0;
--- a/src/hotspot/share/code/nmethod.hpp	Thu May 02 16:10:02 2019 -0700
+++ b/src/hotspot/share/code/nmethod.hpp	Fri May 03 16:37:19 2019 -0700
@@ -379,7 +379,6 @@
   }
 
   int   comp_level() const                        { return _comp_level; }
-  bool  is_c1() const                             { return CompLevel_simple <= _comp_level && _comp_level <= CompLevel_full_profile; }
   void unlink_from_method(bool acquire_lock);
 
   // Support for oops in scopes and relocs:
--- a/src/hotspot/share/runtime/sharedRuntime.cpp	Thu May 02 16:10:02 2019 -0700
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp	Fri May 03 16:37:19 2019 -0700
@@ -1142,6 +1142,11 @@
     frame stubFrame   = thread->last_frame();
     // Caller-frame is a compiled frame
     frame callerFrame = stubFrame.sender(&reg_map2);
+    bool caller_is_c1 = false;
+
+    if (callerFrame.is_compiled_frame() && !callerFrame.is_deoptimized_frame()) {
+      caller_is_c1 = callerFrame.cb()->is_compiled_by_c1();
+    }
 
     methodHandle callee = attached_method;
     if (callee.is_null()) {
@@ -1150,7 +1155,7 @@
         THROW_(vmSymbols::java_lang_NoSuchMethodException(), nullHandle);
       }
     }
-    if (callee->has_scalarized_args() && callee->method_holder()->is_value()) {
+    if (!caller_is_c1 && callee->has_scalarized_args() && callee->method_holder()->is_value()) {
       // If the receiver is a value type that is passed as fields, no oop is available.
       // Resolve the call without receiver null checking.
       assert(!attached_method.is_null(), "must have attached method");
@@ -1296,7 +1301,7 @@
 #endif
 
   bool is_nmethod = caller_nm->is_nmethod();
-  bool caller_is_c1 = caller_nm->is_c1();
+  bool caller_is_c1 = caller_nm->is_compiled_by_c1();
 
   if (is_virtual) {
     Klass* receiver_klass = NULL;
@@ -1368,7 +1373,7 @@
   CodeBlob* caller_cb = caller_frame.cb();
   guarantee(caller_cb != NULL && caller_cb->is_compiled(), "must be called from compiled method");
   CompiledMethod* caller_nm = caller_cb->as_compiled_method_or_null();
-  *caller_is_c1 = caller_nm->is_c1();
+  *caller_is_c1 = caller_nm->is_compiled_by_c1();
 
   // make sure caller is not getting deoptimized
   // and removed before we are done with it.
@@ -1665,7 +1670,7 @@
                                             receiver_klass,
                                             inline_cache->is_optimized(),
                                             false, caller_nm->is_nmethod(),
-                                            caller_nm->is_c1(),
+                                            caller_nm->is_compiled_by_c1(),
                                             info, CHECK_false);
     if (!inline_cache->set_to_monomorphic(info)) {
       needs_ic_stub_refill = true;
@@ -1760,7 +1765,7 @@
   frame caller_frame = thread->last_frame().sender(&reg_map);
   CodeBlob* cb = caller_frame.cb();
   CompiledMethod* caller_nm = cb->as_compiled_method();
-  caller_is_c1 = caller_nm->is_c1();
+  caller_is_c1 = caller_nm->is_compiled_by_c1();
 
   for (;;) {
     ICRefillVerifier ic_refill_verifier;
@@ -1816,7 +1821,7 @@
     // Check for static or virtual call
     bool is_static_call = false;
     CompiledMethod* caller_nm = CodeCache::find_compiled(pc);
-    caller_is_c1 = caller_nm->is_c1();
+    caller_is_c1 = caller_nm->is_compiled_by_c1();
 
     // Default call_addr is the location of the "basic" call.
     // Determine the address of the call we a reresolving. With
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConventionC1.java	Thu May 02 16:10:02 2019 -0700
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConventionC1.java	Fri May 03 16:37:19 2019 -0700
@@ -279,7 +279,7 @@
 
         @DontInline
         @ForceCompile(compLevel = C1)
-        public final int test76_helper(RefPoint rp2) { // opt_virtual_call
+        public final int final_func(RefPoint rp2) { // opt_virtual_call
             return this.x.n + this.y.n + rp2.x.n + rp2.y.n;
         }
 
@@ -1414,15 +1414,10 @@
     }
     /**/
 
-    /*
-
-
-    // FIXME: when C1 makes opt_virtual_call to RefPoint::test76_helper, method resolution fails with an assert
-
     // C2->C1 invokevirtual via VVEP(RO) (opt_virtual_call)
     @Test(compLevel = C2)
     public int test76(RefPoint rp1, RefPoint rp2) {
-        return rp1.test76_helper(rp2);
+        return rp1.final_func(rp2);
     }
 
     @DontCompile
@@ -1432,19 +1427,16 @@
             RefPoint rp1 = refPointField1;
             RefPoint rp2 = refPointField2;
             int result = test76(rp1, rp2);
-            int n = rp1.test76_helper(rp2);
+            int n = rp1.final_func(rp2);
             Asserts.assertEQ(result, n);
         }
     }
-    /**/
-
-    /*
 
     // C2->C1 invokevirtual, force GC for every allocation when entering a C1 VEP (RefPoint)
     // Same as test56, except we call the VVEP(RO) instead of VEP.
     @Test(compLevel = C2)
     public int test77(RefPoint rp1, RefPoint rp2) {
-        return rp1.test76_helper(rp2);
+        return rp1.final_func(rp2);
     }
 
     @DontCompile
@@ -1460,9 +1452,8 @@
             try (ForceGCMarker m = ForceGCMarker.mark(warmup)) {
                 result = test77(rp1, rp2);
             }
-            int n = rp1.test77_helper(rp2);
+            int n = rp1.final_func(rp2);
             Asserts.assertEQ(result, n);
         }
     }
-    /**/
 }