# HG changeset patch # User iklam # Date 1531182503 25200 # Mon Jul 09 17:28:23 2018 -0700 # Branch lworld # Node ID eb7b4e840d2299c3e2c9eebf39b92e64371ce280 # Parent 90b4035ea0f3972117255f9991d3b934592bcbea 8206140: [lworld] Move return value null checks into the callee Reviewed-by: thartmann diff --git a/src/hotspot/cpu/x86/templateTable_x86.cpp b/src/hotspot/cpu/x86/templateTable_x86.cpp --- a/src/hotspot/cpu/x86/templateTable_x86.cpp +++ b/src/hotspot/cpu/x86/templateTable_x86.cpp @@ -2776,7 +2776,19 @@ __ bind(no_safepoint); } - if (state == atos) { + if (EnableValhalla && state == atos) { + Label not_returning_null_vt; + const Register method = rbx, tmp = rdx; + + __ testl(rax, rax); + __ jcc(Assembler::notZero, not_returning_null_vt); + __ get_method(method); + __ load_unsigned_short(tmp, Address(rbx, Method::flags_offset())); + __ testl(tmp, Method::is_returning_vt_mask()); + __ jcc(Assembler::zero, not_returning_null_vt); + __ call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::deoptimize_caller_frame_for_vt), method); + __ bind(not_returning_null_vt); + if (ValueTypesBufferMaxMemory > 0) { Label notBuffered; diff --git a/src/hotspot/share/interpreter/interpreterRuntime.cpp b/src/hotspot/share/interpreter/interpreterRuntime.cpp --- a/src/hotspot/share/interpreter/interpreterRuntime.cpp +++ b/src/hotspot/share/interpreter/interpreterRuntime.cpp @@ -1837,6 +1837,15 @@ // preparing the same method will be sure to see non-null entry & mirror. IRT_END +IRT_ENTRY(void, InterpreterRuntime::deoptimize_caller_frame_for_vt(JavaThread* thread, Method* callee)) + // Called from within the owner thread, so no need for safepoint + assert(callee->is_returning_vt(), "must be"); + RegisterMap reg_map(thread); + frame last_frame = thread->last_frame(); + frame caller_frame = last_frame.sender(®_map); + Deoptimization::deoptimize_frame(thread, caller_frame.id()); +IRT_END + #if defined(IA32) || defined(AMD64) || defined(ARM) IRT_LEAF(void, InterpreterRuntime::popframe_move_outgoing_args(JavaThread* thread, void* src_address, void* dest_address)) if (src_address == dest_address) { diff --git a/src/hotspot/share/interpreter/interpreterRuntime.hpp b/src/hotspot/share/interpreter/interpreterRuntime.hpp --- a/src/hotspot/share/interpreter/interpreterRuntime.hpp +++ b/src/hotspot/share/interpreter/interpreterRuntime.hpp @@ -189,6 +189,8 @@ static void verify_mdp(Method* method, address bcp, address mdp); #endif // ASSERT static MethodCounters* build_method_counters(JavaThread* thread, Method* m); + + static void deoptimize_caller_frame_for_vt(JavaThread* thread, Method* callee); }; diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -659,6 +659,9 @@ if (!klass->is_value()) { THROW_(vmSymbols::java_lang_IncompatibleClassChangeError(), false); } + if (ss.at_return_type()) { + m->set_is_returning_vt(); + } } } } diff --git a/src/hotspot/share/oops/method.hpp b/src/hotspot/share/oops/method.hpp --- a/src/hotspot/share/oops/method.hpp +++ b/src/hotspot/share/oops/method.hpp @@ -90,7 +90,8 @@ _has_injected_profile = 1 << 4, _running_emcp = 1 << 5, _intrinsic_candidate = 1 << 6, - _reserved_stack_access = 1 << 7 + _reserved_stack_access = 1 << 7, + _is_returning_vt = 1 << 8 }; mutable u2 _flags; @@ -581,7 +582,6 @@ Symbol* klass_name() const; // returns the name of the method holder BasicType result_type() const; // type of the method result 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; } #ifdef ASSERT ValueKlass* returned_value_type(Thread* thread) const; #endif @@ -695,6 +695,7 @@ static ByteSize access_flags_offset() { return byte_offset_of(Method, _access_flags ); } static ByteSize from_compiled_offset() { return byte_offset_of(Method, _from_compiled_entry); } static ByteSize code_offset() { return byte_offset_of(Method, _code); } + static ByteSize flags_offset() { return byte_offset_of(Method, _flags); } static ByteSize method_data_offset() { return byte_offset_of(Method, _method_data); } @@ -896,6 +897,18 @@ _flags = x ? (_flags | _reserved_stack_access) : (_flags & ~_reserved_stack_access); } + static int is_returning_vt_mask() { + return _is_returning_vt; + } + + bool is_returning_vt() const { + return (_flags & _is_returning_vt) != 0; + } + + void set_is_returning_vt() { + _flags |= _is_returning_vt; + } + JFR_ONLY(DEFINE_TRACE_FLAG_ACCESSOR;) ConstMethod::MethodType method_type() const { diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -770,7 +770,7 @@ CallGenerator* cg = NULL; if (is_osr_compilation()) { const TypeTuple *domain = StartOSRNode::osr_domain(); - const TypeTuple *range = TypeTuple::make_range(method()->signature()); + const TypeTuple *range = TypeTuple::make_range(method()); init_tf(TypeFunc::make(domain, range)); StartNode* s = new StartOSRNode(root(), domain); initial_gvn()->set_type_bottom(s); diff --git a/src/hotspot/share/opto/doCall.cpp b/src/hotspot/share/opto/doCall.cpp --- a/src/hotspot/share/opto/doCall.cpp +++ b/src/hotspot/share/opto/doCall.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2018, 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 @@ -669,6 +669,12 @@ 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 (ct == T_VALUETYPE && cg->method()->get_Method()->is_returning_vt()) { + // A NULL ValueType cannot be returned to compiled code. The 'areturn' bytecode + // handler will deoptimize its caller if it is about to return a NULL ValueType. + // (See comments inside TypeTuple::make_range). + sig_type = sig_type->join_speculative(TypePtr::NOTNULL); + } if (arg_type != NULL && !arg_type->higher_equal(sig_type)) { Node* retnode = pop(); Node* cast_obj = _gvn.transform(new CheckCastPPNode(control(), retnode, sig_type)); diff --git a/src/hotspot/share/opto/type.cpp b/src/hotspot/share/opto/type.cpp --- a/src/hotspot/share/opto/type.cpp +++ b/src/hotspot/share/opto/type.cpp @@ -1936,12 +1936,13 @@ //------------------------------make------------------------------------------- // Make a TypeTuple from the range of a method signature -const TypeTuple *TypeTuple::make_range(ciSignature* sig, bool ret_vt_fields) { +const TypeTuple *TypeTuple::make_range(ciMethod* method, bool ret_vt_fields) { + ciSignature* sig = method->signature(); ciType* return_type = sig->return_type(); - return make_range(return_type, ret_vt_fields); -} - -const TypeTuple *TypeTuple::make_range(ciType* return_type, bool ret_vt_fields) { + return make_range(method, return_type, ret_vt_fields); +} + +const TypeTuple *TypeTuple::make_range(ciMethod* method, ciType* return_type, bool ret_vt_fields) { uint arg_cnt = 0; if (ret_vt_fields) { ret_vt_fields = return_type->is_valuetype() && ((ciValueKlass*)return_type)->can_be_returned_as_fields(); @@ -1981,9 +1982,15 @@ pos++; collect_value_fields(vk, field_array, pos); } else { - // Value type returns cannot be NULL - //field_array[TypeFunc::Parms] = get_const_type(return_type)->join_speculative(TypePtr::NOTNULL); - field_array[TypeFunc::Parms] = get_const_type(return_type); + if (method->get_Method()->is_returning_vt()) { + // A NULL ValueType cannot be returned to compiled code. The 'areturn' bytecode + // handler will deoptimize its caller if it is about to return a NULL ValueType. + field_array[TypeFunc::Parms] = get_const_type(return_type)->join_speculative(TypePtr::NOTNULL); + } else { + // We're calling a legacy class's method that is returning a VT but doesn't know about it, so + // it won't do the deoptimization at 'areturn'. + field_array[TypeFunc::Parms] = get_const_type(return_type); + } } break; case T_VOID: @@ -5611,8 +5618,8 @@ domain_sig = TypeTuple::make_domain(method->holder(), method->signature(), false); domain_cc = TypeTuple::make_domain(method->holder(), method->signature(), ValueTypePassFieldsAsArgs); } - const TypeTuple *range_sig = TypeTuple::make_range(method->signature(), false); - const TypeTuple *range_cc = TypeTuple::make_range(method->signature(), ValueTypeReturnedAsFields); + const TypeTuple *range_sig = TypeTuple::make_range(method, false); + const TypeTuple *range_cc = TypeTuple::make_range(method, ValueTypeReturnedAsFields); tf = TypeFunc::make(domain_sig, domain_cc, range_sig, range_cc); C->set_last_tf(method, tf); // fill cache return tf; diff --git a/src/hotspot/share/opto/type.hpp b/src/hotspot/share/opto/type.hpp --- a/src/hotspot/share/opto/type.hpp +++ b/src/hotspot/share/opto/type.hpp @@ -690,8 +690,11 @@ } static const TypeTuple *make( uint cnt, const Type **fields ); - static const TypeTuple *make_range(ciSignature *sig, bool ret_vt_fields = false); - static const TypeTuple *make_range(ciType *ret_type, bool ret_vt_fields = false); + static const TypeTuple *make_range(ciMethod *method, bool ret_vt_fields = false); + static const TypeTuple *make_range(ciMethod *method, ciType *ret_type, bool ret_vt_fields = false); + static const TypeTuple *make_range(ciType *ret_type, bool ret_vt_fields = false) { + return make_range(NULL, ret_type, ret_vt_fields); + } static const TypeTuple *make_domain(ciInstanceKlass* recv, ciSignature *sig, bool vt_fields_as_args = false); // Subroutine call type with space allocated for argument types diff --git a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java --- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java +++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java @@ -38,6 +38,7 @@ * java.base/jdk.experimental.value * @library /testlibrary /test/lib /compiler/whitebox / * @requires os.simpleArch == "x64" + * @build TestLWorld_mismatched * @compile -XDenableValueTypes -XDallowFlattenabilityModifiers TestLWorld.java * @run driver ClassFileInstaller sun.hotspot.WhiteBox jdk.test.lib.Platform * @run main/othervm/timeout=120 -Xbootclasspath/a:. -ea -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions @@ -60,7 +61,8 @@ public static void main(String[] args) throws Throwable { TestLWorld test = new TestLWorld(); - test.run(args, MyValue1.class, MyValue2.class, MyValue2Inline.class, MyValue3.class, MyValue3Inline.class, Test65Value.class); + test.run(args, MyValue1.class, MyValue2.class, MyValue2Inline.class, MyValue3.class, + MyValue3Inline.class, Test65Value.class, TestLWorld_mismatched.class); } // Helper methods @@ -2233,4 +2235,39 @@ int result = test86(); Asserts.assertEquals(result, 0); } + + @DontInline + MyValue1 get_nullField() { + return nullField; + } + + // A callees that returns a VT performs null check (and deoptimizes caller) before returning. + @Test(match = {"CallStaticJava.*TestLWorld::get_nullField compiler/valhalla/valuetypes/MyValue1:NotNull"}, matchCount = {2}) + public void test87() { + try { + valueField1 = get_nullField(); + throw new RuntimeException("NullPointerException expected"); + } catch (NullPointerException e) { + // Expected + } + + nullField = get_nullField(); // should not throw + } + + @DontCompile + public void test87_verifier(boolean warmup) { + test87(); + } + + // A callee that's not aware of VT may return a null to the caller. An + // explicit null check is needed in compiled code. + @Test(failOn = "CallStaticJava.*TestLWorld_mismatched::test88_callee compiler/valhalla/valuetypes/MyValue1:NotNull") + public void test88() { + TestLWorld_mismatched.test88(); + } + + @DontCompile + public void test88_verifier(boolean warmup) { + test88(); + } } diff --git a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld_mismatched.jasm b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld_mismatched.jasm new file mode 100644 --- /dev/null +++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld_mismatched.jasm @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2018, 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. + */ + +// TestLWorld.java calls this class, but this class is not +// aware that MyValue1 is a VT. + +package compiler/valhalla/valuetypes; + +super class TestLWorld_mismatched + version 48:0 +{ + + +Method "":"()V" + stack 1 locals 1 +{ + aload_0; + invokespecial Method java/lang/Object."":"()V"; + return; +} + +static Field x:"Lcompiler/valhalla/valuetypes/MyValue1;"; + +@+compiler/valhalla/valuetypes/ForceInline { } +static Method test88:"()V" + stack 1 locals 0 +{ + invokestatic Method test88_callee:"()Lcompiler/valhalla/valuetypes/MyValue1;"; + putstatic Field x:"Lcompiler/valhalla/valuetypes/MyValue1;"; + return; +} + +@+compiler/valhalla/valuetypes/DontInline { } +static Method test88_callee:"()Lcompiler/valhalla/valuetypes/MyValue1;" + stack 1 locals 0 +{ + aconst_null; + areturn; +} + +} // end Class TestLWorld_mismatched