changeset 47430:3ac981476357 mvt

8189613: [MVT] __Value oop not preserved when safepointing at return Reviewed-by: roland
author thartmann
date Mon, 23 Oct 2017 16:18:53 +0200
parents 3cba963b417f
children b704aecd02e6
files src/hotspot/share/ci/ciValueKlass.cpp src/hotspot/share/jvmci/jvmciCodeInstaller.cpp src/hotspot/share/oops/method.hpp src/hotspot/share/oops/valueKlass.cpp src/hotspot/share/oops/valueKlass.hpp src/hotspot/share/runtime/deoptimization.cpp src/hotspot/share/runtime/safepoint.cpp src/hotspot/share/runtime/sharedRuntime.cpp src/hotspot/share/shark/sharkNativeWrapper.hpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnresolvedValueClass.java
diffstat 10 files changed, 48 insertions(+), 50 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/ci/ciValueKlass.cpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/ci/ciValueKlass.cpp	Mon Oct 23 16:18:53 2017 +0200
@@ -77,7 +77,7 @@
 
 // Can this value type be returned as multiple values?
 bool ciValueKlass::can_be_returned_as_fields() const {
-  GUARDED_VM_ENTRY(return !is__Value() && ValueKlass::cast(get_Klass())->return_regs() != NULL;)
+  GUARDED_VM_ENTRY(return ValueKlass::cast(get_Klass())->can_be_returned_as_fields();)
 }
 
 // When passing a value type's fields as arguments, count the number
--- a/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp	Mon Oct 23 16:18:53 2017 +0200
@@ -1153,7 +1153,7 @@
     OopMap *map = create_oop_map(debug_info, CHECK);
     _debug_recorder->add_safepoint(next_pc_offset, map);
 
-    bool return_oop = hotspot_method.not_null() && getMethodFromHotSpotMethod(hotspot_method())->is_returning_oop();
+    bool return_oop = hotspot_method.not_null() && getMethodFromHotSpotMethod(hotspot_method())->may_return_oop();
 
     record_scope(next_pc_offset, debug_info, CodeInstaller::FullFrame, return_oop, CHECK);
   }
--- a/src/hotspot/share/oops/method.hpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/oops/method.hpp	Mon Oct 23 16:18:53 2017 +0200
@@ -583,9 +583,8 @@
   void compute_size_of_parameters(Thread *thread); // word size of parameters (receiver if any + arguments)
   Symbol* klass_name() const;                    // returns the name of the method holder
   BasicType result_type() const;                 // type of the method result
-  bool is_returning_oop() const                  { BasicType r = result_type(); return (r == T_OBJECT || r == T_ARRAY); }
+  bool may_return_oop() const                    { BasicType r = result_type(); return (r == T_OBJECT || r == T_ARRAY ||  r == T_VALUETYPE); }
   bool is_returning_vt() const                   { BasicType r = result_type(); return r == T_VALUETYPE; }
-  bool is_returning_fp() const                   { BasicType r = result_type(); return (r == T_FLOAT || r == T_DOUBLE); }
 #ifdef ASSERT
   ValueKlass* returned_value_type(Thread* thread) const;
 #endif
--- a/src/hotspot/share/oops/valueKlass.cpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/oops/valueKlass.cpp	Mon Oct 23 16:18:53 2017 +0200
@@ -51,8 +51,7 @@
 }
 
 int ValueKlass::raw_value_byte_size() const {
-  assert(this != SystemDictionary::___Value_klass(),
-      "This is not the value type klass you are looking for");
+  assert(!is__Value(), "This is not the value type klass you are looking for");
   int heapOopAlignedSize = nonstatic_field_size() << LogBytesPerHeapOop;
   // If bigger than 64 bits or needs oop alignment, then use jlong aligned
   // which for values should be jlong aligned, asserts in raw_field_copy otherwise
@@ -412,19 +411,12 @@
   }
 }
 
-// Create handles for all oop fields returned in registers that are
-// going to be live across a safepoint.
-bool ValueKlass::save_oop_results(RegisterMap& reg_map, GrowableArray<Handle>& handles) const {
-  if (ValueTypeReturnedAsFields) {
-    if (return_regs() != NULL) {
-      save_oop_fields(reg_map, handles);
-      return true;
-    }
-  }
-  return false;
+// Can this value type be returned as multiple values?
+bool ValueKlass::can_be_returned_as_fields() const {
+  return !is__Value() && (return_regs() != NULL);
 }
 
-// Same as above but with pre-computed return convention
+// Create handles for all oop fields returned in registers that are going to be live across a safepoint
 void ValueKlass::save_oop_fields(const RegisterMap& reg_map, GrowableArray<Handle>& handles) const {
   Thread* thread = Thread::current();
   const Array<SigEntry>* sig_vk = extended_sig();
@@ -585,7 +577,8 @@
   return new_vt;
 }
 
-ValueKlass* ValueKlass::returned_value_type(const RegisterMap& map) {
+// Check the return register for a ValueKlass oop
+ValueKlass* ValueKlass::returned_value_klass(const RegisterMap& map) {
   BasicType bt = T_METADATA;
   VMRegPair pair;
   int nb = SharedRuntime::java_return_convention(&bt, &pair, 1);
@@ -594,9 +587,18 @@
   address loc = map.location(pair.first());
   intptr_t ptr = *(intptr_t*)loc;
   if (is_set_nth_bit(ptr, 0)) {
+    // Oop is tagged, must be a ValueKlass oop
     clear_nth_bit(ptr, 0);
     assert(Metaspace::contains((void*)ptr), "should be klass");
-    return (ValueKlass*)ptr;
+    ValueKlass* vk = (ValueKlass*)ptr;
+    assert(vk->can_be_returned_as_fields(), "must be able to return as fields");
+    return vk;
   }
+#ifdef ASSERT
+  // Oop is not tagged, must be a valid oop
+  if (VerifyOops) {
+    oop((HeapWord*)ptr)->verify();
+  }
+#endif
   return NULL;
 }
--- a/src/hotspot/share/oops/valueKlass.hpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/oops/valueKlass.hpp	Mon Oct 23 16:18:53 2017 +0200
@@ -116,6 +116,7 @@
  public:
   // Type testing
   bool is_value_slow() const        { return true; }
+  bool is__Value() const { return (this == SystemDictionary::___Value_klass()); }
 
   // Casting from Klass*
   static ValueKlass* cast(Klass* k) {
@@ -208,18 +209,18 @@
   // calling convention support
   void initialize_calling_convention();
   Array<SigEntry>* extended_sig() const {
-    assert(this != SystemDictionary::___Value_klass(), "make no sense for __Value");
+    assert(!is__Value(), "make no sense for __Value");
     return *((Array<SigEntry>**)adr_extended_sig());
   }
   Array<VMRegPair>* return_regs() const {
-    assert(this != SystemDictionary::___Value_klass(), "make no sense for __Value");
+    assert(!is__Value(), "make no sense for __Value");
     return *((Array<VMRegPair>**)adr_return_regs());
   }
+  bool can_be_returned_as_fields() const;
   void save_oop_fields(const RegisterMap& map, GrowableArray<Handle>& handles) const;
-  bool save_oop_results(RegisterMap& map, GrowableArray<Handle>& handles) const;
   void restore_oop_results(RegisterMap& map, GrowableArray<Handle>& handles) const;
   oop realloc_result(const RegisterMap& reg_map, const GrowableArray<Handle>& handles, bool buffered, TRAPS);
-  static ValueKlass* returned_value_type(const RegisterMap& reg_map);
+  static ValueKlass* returned_value_klass(const RegisterMap& reg_map);
 
   // pack and unpack handlers. Need to be loadable from generated code
   // so at a fixed offset from the base of the klass pointer.
--- a/src/hotspot/share/runtime/deoptimization.cpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/runtime/deoptimization.cpp	Mon Oct 23 16:18:53 2017 +0200
@@ -220,16 +220,11 @@
       // of all oop return values.
       GrowableArray<Handle> return_oops;
       ValueKlass* vk = NULL;
-      if (save_oop_result) {
-        if (scope->return_vt()) {
-          vk = ValueKlass::returned_value_type(map);
-          if (vk != NULL) {
-            bool success = vk->save_oop_results(map, return_oops);
-            assert(success, "found klass ptr being returned: saving oops can't fail");
-            save_oop_result = false;
-          } else {
-            vk = NULL;
-          }
+      if (save_oop_result && scope->return_vt()) {
+        vk = ValueKlass::returned_value_klass(map);
+        if (vk != NULL) {
+          vk->save_oop_fields(map, return_oops);
+          save_oop_result = false;
         }
       }
       if (save_oop_result) {
--- a/src/hotspot/share/runtime/safepoint.cpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/runtime/safepoint.cpp	Mon Oct 23 16:18:53 2017 +0200
@@ -1100,20 +1100,20 @@
     ResourceMark rm;
     // See if return type is an oop.
     Method* method = nm->method();
-    bool return_oop = method->is_returning_oop();
-    
+    bool return_oop = method->may_return_oop();
+
     GrowableArray<Handle> return_values;
     ValueKlass* vk = NULL;
-    if (!return_oop && method->is_returning_vt()) {
-      // We're at a safepoint at the return of a method that returns
-      // multiple values. We must make sure we preserve the oop values
-      // across the safepoint.
-      vk = ValueKlass::returned_value_type(map);
-      assert(vk == NULL || vk == method->returned_value_type(thread()) ||
-             method->returned_value_type(thread()) == SystemDictionary::___Value_klass(), "Bad value klass");
-      if (vk != NULL && !vk->save_oop_results(map, return_values)) {
-        return_oop = true;
-        vk = NULL;
+    if (method->is_returning_vt() && ValueTypeReturnedAsFields) {
+      // Check if value type is returned as fields
+      vk = ValueKlass::returned_value_klass(map);
+      if (vk != NULL) {
+        // We're at a safepoint at the return of a method that returns
+        // multiple values. We must make sure we preserve the oop values
+        // across the safepoint.
+        assert(vk == method->returned_value_type(thread()), "bad value klass");
+        vk->save_oop_fields(map, return_values);
+        return_oop = false;
       }
     }
 
--- a/src/hotspot/share/runtime/sharedRuntime.cpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp	Mon Oct 23 16:18:53 2017 +0200
@@ -2690,7 +2690,7 @@
       if (!method->is_static()) {  // Pass in receiver first
         if (holder->is_value()) {
           ValueKlass* vk = ValueKlass::cast(holder);
-          if (!ValueTypePassFieldsAsArgs || (vk == SystemDictionary::___Value_klass())) {
+          if (!ValueTypePassFieldsAsArgs || vk->is__Value()) {
             // If we don't pass value types as arguments or if the holder of
             // the method is __Value, we must pass a reference.
             sig_extended.push(SigEntry(T_VALUETYPEPTR));
@@ -2953,7 +2953,7 @@
             Handle protection_domain(THREAD, method->method_holder()->protection_domain());
             Klass* k = ss.as_klass(class_loader, protection_domain, SignatureStream::ReturnNull, THREAD);
             assert(k != NULL && !HAS_PENDING_EXCEPTION, "can't resolve klass");
-            assert(k == SystemDictionary::___Value_klass(), "other values not supported");
+            assert(ValueKlass::cast(k)->is__Value(), "other values not supported");
           }
 #endif
           bt = T_VALUETYPEPTR;
@@ -3467,7 +3467,7 @@
   frame callerFrame = stubFrame.sender(&reg_map);
 
 #ifdef ASSERT
-  ValueKlass* verif_vk = ValueKlass::returned_value_type(reg_map);
+  ValueKlass* verif_vk = ValueKlass::returned_value_klass(reg_map);
 #endif
 
   if (!is_set_nth_bit(res, 0)) {
@@ -3507,7 +3507,7 @@
     methodHandle callee = inv.static_target(thread);
     assert(!thread->has_pending_exception(), "call resolution should work");
     ValueKlass* verif_vk2 = callee->returned_value_type(thread);
-    assert(verif_vk == verif_vk2 || verif_vk2 == SystemDictionary::___Value_klass(), "Bad value klass");
+    assert(verif_vk == verif_vk2 || verif_vk2->is__Value(), "Bad value klass");
 #endif
   }
   JRT_BLOCK_END;
--- a/src/hotspot/share/shark/sharkNativeWrapper.hpp	Fri Oct 20 16:06:21 2017 +0200
+++ b/src/hotspot/share/shark/sharkNativeWrapper.hpp	Mon Oct 23 16:18:53 2017 +0200
@@ -99,7 +99,7 @@
     return target()->is_synchronized();
   }
   bool is_returning_oop() const {
-    return target()->is_returning_oop();
+    return target()->may_return_oop();
   }
 
   // The LLVM function we are building.
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnresolvedValueClass.java	Fri Oct 20 16:06:21 2017 +0200
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnresolvedValueClass.java	Mon Oct 23 16:18:53 2017 +0200
@@ -25,6 +25,7 @@
  * @test
  * @bug 8187679
  * @summary The VM should exit gracefully when unable to resolve a value type argument
+ * @requires vm.compMode != "Xint"
  * @library /test/lib
  * @compile -XDenableValueTypes TestUnresolvedValueClass.java
  * @run main/othervm -XX:+EnableValhalla TestUnresolvedValueClass