changeset 13528:f017570123b2 mvt

8185858: [MVT] "must constrain OSR typestate" crash Reviewed-by: thartmann
author roland
date Mon, 21 Aug 2017 14:05:14 +0200
parents d9b9c6c9c4b2
children 958228979fe8
files src/cpu/x86/vm/sharedRuntime_x86_64.cpp src/share/vm/c1/c1_LinearScan.cpp src/share/vm/classfile/classFileParser.cpp src/share/vm/opto/connode.cpp src/share/vm/opto/parse1.cpp src/share/vm/opto/type.cpp src/share/vm/runtime/sharedRuntime.cpp src/share/vm/runtime/signature.cpp src/share/vm/utilities/globalDefinitions.cpp src/share/vm/utilities/globalDefinitions.hpp test/compiler/valhalla/valuetypes/ValueTypeTestBench.java
diffstat 11 files changed, 83 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/src/cpu/x86/vm/sharedRuntime_x86_64.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/cpu/x86/vm/sharedRuntime_x86_64.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -485,7 +485,7 @@
     case T_OBJECT:
     case T_ARRAY:
     case T_ADDRESS:
-    case T_VALUETYPE: // just treat as ref for now
+    case T_VALUETYPEPTR:
       if (int_args < Argument::n_int_register_parameters_j) {
         regs[i].set2(INT_ArgReg[int_args++]->as_VMReg());
       } else {
@@ -567,6 +567,7 @@
     case T_ARRAY:
     case T_ADDRESS:
     case T_METADATA:
+    case T_VALUETYPEPTR:
       if (int_args < Argument::n_int_register_parameters_j+1) {
         regs[i].set2(INT_ArgReg[int_args]->as_VMReg());
         int_args++;
--- a/src/share/vm/c1/c1_LinearScan.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/c1/c1_LinearScan.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -61,9 +61,9 @@
 
 // Map BasicType to spill size in 32-bit words, matching VMReg's notion of words
 #ifdef _LP64
-static int type2spill_size[T_CONFLICT+1]={ -1, 0, 0, 0, 1, 1, 1, 2, 1, 1, 1, 2, 2, 2, 0, 2,  1, 2, 1, -1};
+static int type2spill_size[T_CONFLICT+1]={ -1, 0, 0, 0, 1, 1, 1, 2, 1, 1, 1, 2, 2, 2, 0, 2,  1, 2, 1, 2, -1};
 #else
-static int type2spill_size[T_CONFLICT+1]={ -1, 0, 0, 0, 1, 1, 1, 2, 1, 1, 1, 2, 1, 1, 0, 1, -1, 1, 1, -1};
+static int type2spill_size[T_CONFLICT+1]={ -1, 0, 0, 0, 1, 1, 1, 2, 1, 1, 1, 2, 1, 1, 0, 1, -1, 1, 1, 1, -1};
 #endif
 
 
--- a/src/share/vm/classfile/classFileParser.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/classfile/classFileParser.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -1464,7 +1464,8 @@
   BAD_ALLOCATION_TYPE, // T_NARROWOOP   = 17,
   BAD_ALLOCATION_TYPE, // T_METADATA    = 18,
   BAD_ALLOCATION_TYPE, // T_NARROWKLASS = 19,
-  BAD_ALLOCATION_TYPE, // T_CONFLICT    = 20,
+  BAD_ALLOCATION_TYPE, // T_VALUETYPEPTR= 20,
+  BAD_ALLOCATION_TYPE, // T_CONFLICT    = 21,
   BAD_ALLOCATION_TYPE, // 0
   BAD_ALLOCATION_TYPE, // 1
   BAD_ALLOCATION_TYPE, // 2
@@ -1485,7 +1486,8 @@
   BAD_ALLOCATION_TYPE, // T_NARROWOOP   = 17,
   BAD_ALLOCATION_TYPE, // T_METADATA    = 18,
   BAD_ALLOCATION_TYPE, // T_NARROWKLASS = 19,
-  BAD_ALLOCATION_TYPE, // T_CONFLICT    = 20,
+  BAD_ALLOCATION_TYPE, // T_VALUETYPEPTR= 20,
+  BAD_ALLOCATION_TYPE, // T_CONFLICT    = 21,
 };
 
 static FieldAllocationType basic_type_to_atype(bool is_static, BasicType type) {
--- a/src/share/vm/opto/connode.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/opto/connode.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -50,6 +50,7 @@
   case T_FLOAT:       return new ConFNode( t->is_float_constant() );
   case T_DOUBLE:      return new ConDNode( t->is_double_constant() );
   case T_VOID:        return new ConNode ( Type::TOP );
+  case T_VALUETYPEPTR:
   case T_OBJECT:      return new ConPNode( t->is_ptr() );
   case T_ARRAY:       return new ConPNode( t->is_aryptr() );
   case T_ADDRESS:     return new ConPNode( t->is_ptr() );
--- a/src/share/vm/opto/parse1.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/opto/parse1.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -130,6 +130,10 @@
     l = ValueTypeNode::make(gvn(), mem, l);
     break;
   }
+  case T_VALUETYPEPTR: {
+    l = new LoadPNode(ctl, mem, adr, TypeRawPtr::BOTTOM, TypeValueTypePtr::NOTNULL, MemNode::unordered);
+    break;
+  }
   case T_LONG:
   case T_DOUBLE: {
     // Since arguments are in reverse order, the argument address 'adr'
--- a/src/share/vm/opto/type.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/opto/type.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -133,7 +133,7 @@
   { Bad,             T_ADDRESS,    "rawptr:",       false, Op_RegP,              relocInfo::none          },  // RawPtr
   { Bad,             T_OBJECT,     "oop:",          true,  Op_RegP,              relocInfo::oop_type      },  // OopPtr
   { Bad,             T_OBJECT,     "inst:",         true,  Op_RegP,              relocInfo::oop_type      },  // InstPtr
-  { Bad,             T_OBJECT,     "valueptr:",     true,  Op_RegP,              relocInfo::oop_type      },  // ValueTypePtr
+  { Bad,             T_VALUETYPEPTR,"valueptr:",     true,  Op_RegP,              relocInfo::oop_type      },  // ValueTypePtr
   { Bad,             T_OBJECT,     "ary:",          true,  Op_RegP,              relocInfo::oop_type      },  // AryPtr
   { Bad,             T_METADATA,   "metadata:",     false, Op_RegP,              relocInfo::metadata_type },  // MetadataPtr
   { Bad,             T_METADATA,   "klass:",        false, Op_RegP,              relocInfo::metadata_type },  // KlassPtr
--- a/src/share/vm/runtime/sharedRuntime.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/runtime/sharedRuntime.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -2336,6 +2336,7 @@
         // between a T_VALUETYPE and a T_OBJECT in the signature.
         return ValueTypePassFieldsAsArgs ? in : adapter_encoding(T_OBJECT, false);
       }
+      case T_VALUETYPEPTR:
       case T_OBJECT:
       case T_ARRAY:
         // In other words, we assume that any register good enough for
@@ -2706,12 +2707,8 @@
           ValueKlass* vk = ValueKlass::cast(holder);
           if (vk == SystemDictionary::___Value_klass()) {
             // If the holder of the method is __Value, we must pass a
-            // reference. FIXME: this shouldn't be T_OBJECT as a value
-            // type reference is not necessarily an oop. Ideally we
-            // would use T_VALUETYPE but we can't because T_VALUETYPE
-            // is used here as a marker right before the list of
-            // fields for the value type.
-            sig_extended.push(SigEntry(T_OBJECT));
+            // reference.
+            sig_extended.push(SigEntry(T_VALUETYPEPTR));
           } else {
             const Array<SigEntry>* sig_vk = vk->extended_sig();
             sig_extended.appendAll(sig_vk);
@@ -2727,7 +2724,7 @@
           if (name == vmSymbols::java_lang____Value()) {
             assert(method->is_compiled_lambda_form() || method->is_method_handle_intrinsic(),
                    "should not use __Value for a value type argument");
-            sig_extended.push(SigEntry(T_OBJECT));
+            sig_extended.push(SigEntry(T_VALUETYPEPTR));
           } else {
             // Method handle intrinsics with a __Value argument may be created during
             // compilation. Only do a full system dictionary lookup if the argument name
@@ -2962,7 +2959,19 @@
         sig_bt[i++] = T_OBJECT;
       SignatureStream ss(method->signature());
       for (; !ss.at_return_type(); ss.next()) {
-        sig_bt[i++] = ss.type();  // Collect remaining bits of signature
+        BasicType bt = ss.type();
+        if  (bt == T_VALUETYPE) {
+#ifdef ASSERT
+          Thread* THREAD = Thread::current();
+          Handle class_loader(THREAD, method->method_holder()->class_loader());
+          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");
+#endif
+          bt = T_VALUETYPEPTR;
+        }
+        sig_bt[i++] = bt;  // Collect remaining bits of signature
         if (ss.type() == T_LONG || ss.type() == T_DOUBLE)
           sig_bt[i++] = T_VOID;   // Longs & doubles take 2 Java slots
       }
--- a/src/share/vm/runtime/signature.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/runtime/signature.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -552,7 +552,11 @@
   int j = 0;
   for (int i = 0; i < sig_extended.length(); i++) {
     if (!skip_vt) {
-      sig_bt_cc[j++] = sig_extended.at(i)._bt;
+      BasicType bt = sig_extended.at(i)._bt;
+      if (bt == T_VALUETYPE) {
+        bt = T_VALUETYPEPTR;
+      }
+      sig_bt_cc[j++] = bt;
     } else if (sig_extended.at(i)._bt != T_VALUETYPE &&
                (sig_extended.at(i)._bt != T_VOID ||
                 sig_extended.at(i-1)._bt == T_LONG ||
--- a/src/share/vm/utilities/globalDefinitions.cpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/utilities/globalDefinitions.cpp	Mon Aug 21 14:05:14 2017 +0200
@@ -119,6 +119,7 @@
       case T_NARROWKLASS: // compressed klass pointer
       case T_CONFLICT:    // might as well support a bottom type
       case T_VOID:        // padding or other unaddressed word
+      case T_VALUETYPEPTR:
         // layout type must map to itself
         assert(vt == ft, "");
         break;
@@ -183,7 +184,7 @@
 
 
 // Map BasicType to signature character
-char type2char_tab[T_CONFLICT+1]={ 0, 0, 0, 0, 'Z', 'C', 'F', 'D', 'B', 'S', 'I', 'J', 'L', '[', 'Q', 'V', 0, 0, 0, 0, 0};
+char type2char_tab[T_CONFLICT+1]={ 0, 0, 0, 0, 'Z', 'C', 'F', 'D', 'B', 'S', 'I', 'J', 'L', '[', 'Q', 'V', 0, 0, 0, 0, 0, 0};
 
 // Map BasicType to Java type name
 const char* type2name_tab[T_CONFLICT+1] = {
@@ -204,6 +205,7 @@
   "*narrowoop*",
   "*metadata*",
   "*narrowklass*",
+  "valuetypeptr",
   "*conflict*"
 };
 
@@ -218,7 +220,7 @@
 }
 
 // Map BasicType to size in words
-int type2size[T_CONFLICT+1]={ -1, 0, 0, 0, 1, 1, 1, 2, 1, 1, 1, 2, 1, 1, 1, 0, 1, 1, 1, 1, -1};
+int type2size[T_CONFLICT+1]={ -1, 0, 0, 0, 1, 1, 1, 2, 1, 1, 1, 2, 1, 1, 1, 0, 1, 1, 1, 1, 1, -1};
 
 BasicType type2field[T_CONFLICT+1] = {
   (BasicType)0,            // 0,
@@ -241,7 +243,8 @@
   T_NARROWOOP,             // T_NARROWOOP= 17,
   T_METADATA,              // T_METADATA = 18,
   T_NARROWKLASS,           // T_NARROWKLASS = 19,
-  T_CONFLICT               // T_CONFLICT = 20,
+  T_VALUETYPEPTR,          // T_VALUETYPEPTR = 20,
+  T_CONFLICT               // T_CONFLICT = 21,
 };
 
 
@@ -266,7 +269,8 @@
   T_NARROWOOP, // T_NARROWOOP  = 17,
   T_METADATA,  // T_METADATA   = 18,
   T_NARROWKLASS, // T_NARROWKLASS  = 19,
-  T_CONFLICT // T_CONFLICT = 20,
+  T_VALUETYPEPTR,// T_VALUETYPEPTR =20,
+  T_CONFLICT // T_CONFLICT = 21,
 };
 
 
@@ -291,7 +295,8 @@
   T_NARROWOOP_aelem_bytes,   // T_NARROWOOP= 17,
   T_OBJECT_aelem_bytes,      // T_METADATA = 18,
   T_NARROWKLASS_aelem_bytes, // T_NARROWKLASS= 19,
-  0                          // T_CONFLICT = 20,
+  T_VALUETYPEPTR_aelem_bytes,// T_VALUETYPE = 20
+  0                          // T_CONFLICT = 21,
 };
 
 #ifdef ASSERT
--- a/src/share/vm/utilities/globalDefinitions.hpp	Mon Aug 21 12:50:40 2017 +0200
+++ b/src/share/vm/utilities/globalDefinitions.hpp	Mon Aug 21 14:05:14 2017 +0200
@@ -593,7 +593,8 @@
   T_NARROWOOP   = 17,
   T_METADATA    = 18,
   T_NARROWKLASS = 19,
-  T_CONFLICT    = 20, // for stack value type with conflicting contents
+  T_VALUETYPEPTR= 20, // the compiler needs a way to identify buffered values
+  T_CONFLICT    = 21, // for stack value type with conflicting contents
   T_ILLEGAL     = 99
 };
 
@@ -687,7 +688,12 @@
 #endif
   T_NARROWOOP_aelem_bytes   = 4,
   T_NARROWKLASS_aelem_bytes = 4,
-  T_VOID_aelem_bytes        = 0
+  T_VOID_aelem_bytes        = 0,
+#ifdef _LP64
+  T_VALUETYPEPTR_aelem_bytes= 4
+#else
+  T_VALUETYPEPTR_aelem_bytes= 8
+#endif
 };
 
 extern int _type2aelembytes[T_CONFLICT+1]; // maps a BasicType to nof bytes used by its array element
--- a/test/compiler/valhalla/valuetypes/ValueTypeTestBench.java	Mon Aug 21 12:50:40 2017 +0200
+++ b/test/compiler/valhalla/valuetypes/ValueTypeTestBench.java	Mon Aug 21 14:05:14 2017 +0200
@@ -3021,6 +3021,34 @@
         Asserts.assertEQ(hash, MyValue2.createWithFieldsInline(rI+test105_i * (b ? 1 : -1), b).hash());
     }
 
+
+    // OSR compilation with __Value local
+    @DontCompile
+    public __Value test106_init() {
+        return MyValue1.createWithFieldsInline(rI, rL);
+    }
+
+    @DontCompile
+    public __Value test106_body() {
+        return MyValue1.createWithFieldsInline(rI, rL);
+    }
+
+    @Test()
+    public __Value test106() throws Throwable {
+        __Value vt = test106_init();
+        for (int i = 0; i < 50_000; i++) {
+            if (i % 2 == 1) {
+                vt = test106_body();
+            }
+        }
+        return vt;
+    }
+
+    @DontCompile
+    public void test106_verifier(boolean warmup) throws Throwable {
+        test106();
+    }
+
     // ========== Test infrastructure ==========
 
     private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox();
@@ -3097,7 +3125,7 @@
     }
 
     public static void main(String[] args) throws Throwable {
-        //tests.values().removeIf(p -> !p.getName().equals("test104")); // Run single test
+        //tests.values().removeIf(p -> !p.getName().equals("test106")); // Run single test
         if (args.length == 0) {
             execute_vm("-XX:+IgnoreUnrecognizedVMOptions", "-XX:-BackgroundCompilation",
                     "-XX:+PrintCompilation", "-XX:+PrintInlining", "-XX:+PrintIdeal", "-XX:+PrintOptoAssembly",