--- old/src/hotspot/share/ci/ciValueKlass.cpp 2017-10-20 11:22:49.199585234 +0200 +++ new/src/hotspot/share/ci/ciValueKlass.cpp 2017-10-20 11:22:48.987585237 +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 --- old/src/hotspot/share/oops/method.hpp 2017-10-20 11:22:49.691585227 +0200 +++ new/src/hotspot/share/oops/method.hpp 2017-10-20 11:22:49.495585230 +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 is_returning_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 --- old/src/hotspot/share/oops/valueKlass.cpp 2017-10-20 11:22:50.099585221 +0200 +++ new/src/hotspot/share/oops/valueKlass.cpp 2017-10-20 11:22:49.927585224 +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& 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& handles) const { Thread* thread = Thread::current(); const Array* 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; } --- old/src/hotspot/share/oops/valueKlass.hpp 2017-10-20 11:22:50.507585216 +0200 +++ new/src/hotspot/share/oops/valueKlass.hpp 2017-10-20 11:22:50.339585218 +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* extended_sig() const { - assert(this != SystemDictionary::___Value_klass(), "make no sense for __Value"); + assert(!is__Value(), "make no sense for __Value"); return *((Array**)adr_extended_sig()); } Array* return_regs() const { - assert(this != SystemDictionary::___Value_klass(), "make no sense for __Value"); + assert(!is__Value(), "make no sense for __Value"); return *((Array**)adr_return_regs()); } + bool can_be_returned_as_fields() const; void save_oop_fields(const RegisterMap& map, GrowableArray& handles) const; - bool save_oop_results(RegisterMap& map, GrowableArray& handles) const; void restore_oop_results(RegisterMap& map, GrowableArray& handles) const; oop realloc_result(const RegisterMap& reg_map, const GrowableArray& 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. --- old/src/hotspot/share/runtime/deoptimization.cpp 2017-10-20 11:22:50.955585210 +0200 +++ new/src/hotspot/share/runtime/deoptimization.cpp 2017-10-20 11:22:50.759585212 +0200 @@ -220,16 +220,11 @@ // of all oop return values. GrowableArray 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) { --- old/src/hotspot/share/runtime/safepoint.cpp 2017-10-20 11:22:51.459585203 +0200 +++ new/src/hotspot/share/runtime/safepoint.cpp 2017-10-20 11:22:51.259585205 +0200 @@ -1101,19 +1101,19 @@ // See if return type is an oop. Method* method = nm->method(); bool return_oop = method->is_returning_oop(); - + GrowableArray 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; } } --- old/src/hotspot/share/runtime/sharedRuntime.cpp 2017-10-20 11:22:51.887585197 +0200 +++ new/src/hotspot/share/runtime/sharedRuntime.cpp 2017-10-20 11:22:51.739585199 +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(®_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; --- old/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnresolvedValueClass.java 2017-10-20 11:22:52.255585192 +0200 +++ new/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnresolvedValueClass.java 2017-10-20 11:22:52.107585194 +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