changeset 51289:d2aa5d494481 lworld lw1_0

8206140: [lworld] Move return value null checks into the callee Reviewed-by: thartmann
author iklam
date Thu, 26 Jul 2018 10:31:02 +0200
parents 90b4035ea0f3
children 03fcb1a761a4 c579d0a3b660
files src/hotspot/cpu/x86/templateTable_x86.cpp src/hotspot/share/interpreter/interpreterRuntime.cpp src/hotspot/share/interpreter/interpreterRuntime.hpp src/hotspot/share/oops/instanceKlass.cpp src/hotspot/share/oops/method.hpp src/hotspot/share/opto/compile.cpp src/hotspot/share/opto/doCall.cpp src/hotspot/share/opto/type.cpp src/hotspot/share/opto/type.hpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld_mismatched.jasm
diffstat 11 files changed, 171 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/cpu/x86/templateTable_x86.cpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/cpu/x86/templateTable_x86.cpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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;
 
--- a/src/hotspot/share/interpreter/interpreterRuntime.cpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/interpreter/interpreterRuntime.cpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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(&reg_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) {
--- a/src/hotspot/share/interpreter/interpreterRuntime.hpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/interpreter/interpreterRuntime.hpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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);
 };
 
 
--- a/src/hotspot/share/oops/instanceKlass.cpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/oops/instanceKlass.cpp	Thu Jul 26 10:31:02 2018 +0200
@@ -659,6 +659,9 @@
             if (!klass->is_value()) {
               THROW_(vmSymbols::java_lang_IncompatibleClassChangeError(), false);
             }
+            if (ss.at_return_type()) {
+              m->set_is_returning_vt();
+            }
           }
         }
       }
--- a/src/hotspot/share/oops/method.hpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/oops/method.hpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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 {
--- a/src/hotspot/share/opto/compile.cpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/opto/compile.cpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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);
--- a/src/hotspot/share/opto/doCall.cpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/opto/doCall.cpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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));
--- a/src/hotspot/share/opto/type.cpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/opto/type.cpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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;
--- a/src/hotspot/share/opto/type.hpp	Wed Jul 25 13:43:53 2018 -0400
+++ b/src/hotspot/share/opto/type.hpp	Thu Jul 26 10:31:02 2018 +0200
@@ -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
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java	Wed Jul 25 13:43:53 2018 -0400
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java	Thu Jul 26 10:31:02 2018 +0200
@@ -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();
+    }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld_mismatched.jasm	Thu Jul 26 10:31:02 2018 +0200
@@ -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 "<init>":"()V"
+	stack 1 locals 1
+{
+		aload_0;
+		invokespecial	Method java/lang/Object."<init>":"()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