changeset 56232:2718fd83971d lworld

8228633: [lworld][c1] method invocation fails if the return type is an unloaded Q type Reviewed-by: thartmann
author iklam
date Sun, 28 Jul 2019 10:05:42 -0700
parents ab1e0ac6b32f
children f7ad2b61ccfe
files src/hotspot/share/c1/c1_LIR.cpp src/hotspot/share/ci/ciSignature.cpp src/hotspot/share/ci/ciSignature.hpp src/hotspot/share/oops/symbol.cpp src/hotspot/share/oops/symbol.hpp src/hotspot/share/runtime/signature.cpp src/hotspot/share/runtime/signature.hpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConventionC1.java
diffstat 8 files changed, 114 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/c1/c1_LIR.cpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/c1/c1_LIR.cpp	Sun Jul 28 10:05:42 2019 -0700
@@ -1064,7 +1064,7 @@
 
 bool LIR_OpJavaCall::maybe_return_as_fields(ciValueKlass** vk_ret) const {
   if (ValueTypeReturnedAsFields) {
-    if (method()->signature()->returns_never_null()) {
+    if (method()->signature()->maybe_returns_never_null()) {
       ciType* return_type = method()->return_type();
       if (return_type->is_valuetype()) {
         ciValueKlass* vk = return_type->as_value_klass();
@@ -1074,6 +1074,12 @@
           }
           return true;
         }
+      } else {
+        assert(return_type->is_instance_klass() && !return_type->as_instance_klass()->is_loaded(), "must be");
+        if (vk_ret != NULL) {
+          *vk_ret = NULL;
+        }
+        return true;
       }
     } else if (is_method_handle_invoke()) {
       BasicType bt = method()->return_type()->basic_type();
--- a/src/hotspot/share/ci/ciSignature.cpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/ci/ciSignature.cpp	Sun Jul 28 10:05:42 2019 -0700
@@ -134,6 +134,22 @@
 }
 
 // ------------------------------------------------------------------
+// ciSignature::maybe_return_never_null
+//
+// True if we statically know that the return value is never null, or
+// if the return type has a Q signature but is not yet loaded, in which case
+// it could be a never-null type.
+bool ciSignature::maybe_returns_never_null() const {
+  ciType* ret_type = _types->at(_count);
+  if (ret_type->is_never_null()) {
+    return true;
+  } else if (ret_type->is_instance_klass() && !ret_type->as_instance_klass()->is_loaded()) {
+    GUARDED_VM_ENTRY(if (get_symbol()->is_Q_method_signature()) { return true; })
+  }
+  return false;
+}
+
+// ------------------------------------------------------------------
 // ciSignature::never_null_at
 //
 // True if we statically know that the argument at 'index' is never null.
--- a/src/hotspot/share/ci/ciSignature.hpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/ci/ciSignature.hpp	Sun Jul 28 10:05:42 2019 -0700
@@ -61,6 +61,7 @@
   ciType*   return_type() const;
   ciType*   type_at(int index) const;
   bool      returns_never_null() const;
+  bool      maybe_returns_never_null() const;
   bool      is_never_null_at(int index) const;
 
   int       size() const                         { return _size; }
--- a/src/hotspot/share/oops/symbol.cpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/oops/symbol.cpp	Sun Jul 28 10:05:42 2019 -0700
@@ -131,6 +131,19 @@
   return false;
 }
 
+bool Symbol::is_Q_method_signature() const {
+  assert(SignatureVerifier::is_valid_method_signature(this), "must be");
+  int len = utf8_length();
+  if (len > 4 && char_at(0) == '(') {
+    for (int i=1; i<len-3; i++) { // Must end with ")Qx;", where x is at least one character or more.
+      if (char_at(i) == ')' && char_at(i+1) == 'Q') {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 bool Symbol::is_Q_singledim_array_signature() const {
   return utf8_length() > 3 && char_at(0) == '[' && char_at(1) == 'Q' && ends_with(';');
 }
--- a/src/hotspot/share/oops/symbol.hpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/oops/symbol.hpp	Sun Jul 28 10:05:42 2019 -0700
@@ -231,6 +231,7 @@
 
   bool is_Q_signature() const;
   bool is_Q_array_signature() const;
+  bool is_Q_method_signature() const;
   bool is_Q_singledim_array_signature() const;
   Symbol* fundamental_name(TRAPS);
   bool is_same_fundamental_type(Symbol*) const;
--- a/src/hotspot/share/runtime/signature.cpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/runtime/signature.cpp	Sun Jul 28 10:05:42 2019 -0700
@@ -466,7 +466,7 @@
 }
 
 #ifdef ASSERT
-bool SignatureVerifier::is_valid_method_signature(Symbol* sig) {
+bool SignatureVerifier::is_valid_method_signature(const Symbol* sig) {
   const char* method_sig = (const char*)sig->bytes();
   ssize_t len = sig->utf8_length();
   ssize_t index = 0;
@@ -489,7 +489,7 @@
   return false;
 }
 
-bool SignatureVerifier::is_valid_type_signature(Symbol* sig) {
+bool SignatureVerifier::is_valid_type_signature(const Symbol* sig) {
   const char* type_sig = (const char*)sig->bytes();
   ssize_t len = sig->utf8_length();
   return (type_sig != NULL && len >= 1 &&
--- a/src/hotspot/share/runtime/signature.hpp	Thu Jul 25 13:34:42 2019 -0700
+++ b/src/hotspot/share/runtime/signature.hpp	Sun Jul 28 10:05:42 2019 -0700
@@ -484,8 +484,8 @@
 #ifdef ASSERT
 class SignatureVerifier : public StackObj {
   public:
-    static bool is_valid_method_signature(Symbol* sig);
-    static bool is_valid_type_signature(Symbol* sig);
+    static bool is_valid_method_signature(const Symbol* sig);
+    static bool is_valid_type_signature(const Symbol* sig);
   private:
     static ssize_t is_valid_type(const char*, ssize_t);
 };
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConventionC1.java	Thu Jul 25 13:34:42 2019 -0700
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConventionC1.java	Sun Jul 28 10:05:42 2019 -0700
@@ -44,7 +44,7 @@
 
     @Override
     public int getNumScenarios() {
-        return 4;
+        return 5;
     }
 
     @Override
@@ -56,18 +56,24 @@
                 "-XX:CICompilerCount=2"
             };
         case 1: return new String[] {
+                // Default: both C1 and C2 are enabled, tiered compilation enabled
+                "-XX:+EnableValhallaC1",
+                "-XX:CICompilerCount=2",
+                "-XX:+StressValueTypeReturnedAsFields"
+            };
+        case 2: return new String[] {
                 // Same as above, but flip all the compLevel=C1 and compLevel=C2, so we test
                 // the compliment of the above scenario.
                 "-XX:+EnableValhallaC1",
                 "-XX:CICompilerCount=2",
                 "-DFlipC1C2=true"
             };
-        case 2: return new String[] {
+        case 3: return new String[] {
                 // Only C1. Tiered compilation disabled.
                 "-XX:+EnableValhallaC1",
                 "-XX:TieredStopAtLevel=1",
             };
-        case 3: return new String[] {
+        case 4: return new String[] {
                 // Only C2.
                 "-XX:-EnableValhallaC1",
                 "-XX:TieredStopAtLevel=4",
@@ -2116,4 +2122,67 @@
             Asserts.assertEQ(y, result.y);
         }
     }
+
+    @Test(compLevel = C1)
+    public void test103() {
+        // when this method is compiled by C1, the Test103Value class is not yet loaded.
+        test103_v = new Test103Value(); // invokestatic "Test103Value.<init>()QTest103Value;"
+    }
+
+    static inline class Test103Value {
+        int x = rI;
+    }
+
+    static Object test103_v;
+
+    @DontCompile
+    public void test103_verifier(boolean warmup) {
+        if (warmup) {
+            // Make sure test103() is compiled before Test103Value is loaded
+            return;
+        }
+        test103();
+        Test103Value v = (Test103Value)test103_v;
+        Asserts.assertEQ(v.x, rI);
+    }
+
+
+    // Same as test103, but with an inline class that's too big to return as fields.
+    @Test(compLevel = C1)
+    public void test104() {
+        // when this method is compiled by C1, the Test104Value class is not yet loaded.
+        test104_v = new Test104Value(); // invokestatic "Test104Value.<init>()QTest104Value;"
+    }
+
+    static inline class Test104Value {
+        long x0 = rL;
+        long x1 = rL;
+        long x2 = rL;
+        long x3 = rL;
+        long x4 = rL;
+        long x5 = rL;
+        long x6 = rL;
+        long x7 = rL;
+        long x8 = rL;
+        long x9 = rL;
+        long xa = rL;
+        long xb = rL;
+        long xc = rL;
+        long xd = rL;
+        long xe = rL;
+        long xf = rL;
+    }
+
+    static Object test104_v;
+
+    @DontCompile
+    public void test104_verifier(boolean warmup) {
+        if (warmup) {
+            // Make sure test104() is compiled before Test104Value is loaded
+            return;
+        }
+        test104();
+        Test104Value v = (Test104Value)test104_v;
+        Asserts.assertEQ(v.x0, rL);
+    }
 }