changeset 53180:b890823b8dc6 lworld

8214689: [lworld][c1] aaload cannot handle unloaded class Reviewed-by: thartmann
author iklam
date Tue, 11 Dec 2018 15:26:06 -0800
parents 5853bd189392
children 36cc81c429f5
files src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp src/hotspot/share/c1/c1_GraphBuilder.cpp src/hotspot/share/c1/c1_Instruction.cpp src/hotspot/share/c1/c1_LIRGenerator.cpp src/hotspot/share/c1/c1_LIRGenerator.hpp src/hotspot/share/ci/ciEnv.cpp src/hotspot/share/ci/ciEnv.hpp src/hotspot/share/ci/ciObjectFactory.cpp src/hotspot/share/ci/ciObjectFactory.hpp src/hotspot/share/ci/ciValueArrayKlass.cpp src/hotspot/share/ci/ciValueArrayKlass.hpp src/hotspot/share/ci/ciValueKlass.hpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnloadedValueTypeArray.java
diffstat 13 files changed, 229 insertions(+), 65 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/cpu/x86/c1_LIRGenerator_x86.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -1291,7 +1291,6 @@
   }
   klass2reg_with_patching(klass_reg, obj, patching_info);
   if (obj->is_value_array_klass()) {
-    // This check is valid even if the class is not yet loaded, because the class has a "Q" signature.
     __ allocate_array(reg, len, tmp1, tmp2, tmp3, tmp4, T_VALUETYPE, klass_reg, slow_path);
   } else {
     __ allocate_array(reg, len, tmp1, tmp2, tmp3, tmp4, T_OBJECT, klass_reg, slow_path);
--- a/src/hotspot/share/c1/c1_GraphBuilder.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/c1/c1_GraphBuilder.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -982,6 +982,7 @@
 void GraphBuilder::load_indexed(BasicType type) {
   // In case of in block code motion in range check elimination
   ValueStack* state_before = copy_state_indexed_access();
+  ValueStack* deopt_state = copy_state_before();
   compilation()->set_has_access_indexed(true);
   Value index = ipop();
   Value array = apop();
@@ -993,23 +994,30 @@
   }
 
   if (array->is_flattened_array()) {
-    ciType* array_type = array->declared_type();
-    ciValueKlass* elem_klass = array_type->as_value_array_klass()->element_klass()->as_value_klass();
-    NewValueTypeInstance* new_instance = new NewValueTypeInstance(elem_klass, state_before, false);
-    _memory->new_instance(new_instance);
-    apush(append_split(new_instance));
-    LoadIndexed* load_indexed = new LoadIndexed(array, index, length, type, state_before);
-    load_indexed->set_vt(new_instance);
-    append(load_indexed);
-  } else {
-    push(as_ValueType(type), append(new LoadIndexed(array, index, length, type, state_before)));
+    if (array->declared_type()->is_loaded()) {
+      ciType* array_type = array->declared_type();
+      ciValueKlass* elem_klass = array_type->as_value_array_klass()->element_klass()->as_value_klass();
+      NewValueTypeInstance* new_instance = new NewValueTypeInstance(elem_klass, state_before, false);
+      _memory->new_instance(new_instance);
+      apush(append_split(new_instance));
+      LoadIndexed* load_indexed = new LoadIndexed(array, index, length, type, state_before);
+      load_indexed->set_vt(new_instance);
+      append(load_indexed);
+      return;
+    } else {
+      // Value array access may be deoptimized. Need full "before" states.
+      state_before = deopt_state;
+    }
   }
+
+  push(as_ValueType(type), append(new LoadIndexed(array, index, length, type, state_before)));
 }
 
 
 void GraphBuilder::store_indexed(BasicType type) {
   // In case of in block code motion in range check elimination
   ValueStack* state_before = copy_state_indexed_access();
+  ValueStack* deopt_state = copy_state_before();
   compilation()->set_has_access_indexed(true);
   Value value = pop(as_ValueType(type));
   Value index = ipop();
@@ -1032,6 +1040,11 @@
   } else if (type == T_BYTE) {
     check_boolean = true;
   }
+
+  if (array->is_flattened_array() && !array_type->is_loaded()) {
+    // Value array access may be deoptimized. Need full "before" states.
+    state_before = deopt_state;
+  }
   StoreIndexed* result = new StoreIndexed(array, index, length, type, value, state_before, check_boolean);
   append(result);
   _memory->store_value(value);
--- a/src/hotspot/share/c1/c1_Instruction.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/c1/c1_Instruction.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -117,10 +117,13 @@
 bool Instruction::is_flattened_array() const {
   if (ValueArrayFlatten) {
     ciType* type = declared_type();
-    if (type != NULL &&
-        type->is_value_array_klass() &&
-        type->as_value_array_klass()->element_klass()->as_value_klass()->flatten_array()) {
-    return true;
+    if (type != NULL && type->is_value_array_klass()) {
+      ciValueKlass* element_klass = type->as_value_array_klass()->element_klass()->as_value_klass();
+      if (!element_klass->is_loaded() || element_klass->flatten_array()) {
+        // Assume that all unloaded value arrays are not flattenable. If they
+        // turn out to be flattenable, we deoptimize on aaload/aastore.
+        return true;
+      }
     }
   }
 
--- a/src/hotspot/share/c1/c1_LIRGenerator.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -1565,6 +1565,8 @@
   // Find the starting address of the source (inside the array)
   ciType* array_type = array.value()->declared_type();
   ciValueArrayKlass* value_array_klass = array_type->as_value_array_klass();
+  assert(value_array_klass->is_loaded(), "must be");
+
   ciValueKlass* elem_klass = value_array_klass->element_klass()->as_value_klass();
   int array_header_size = value_array_klass->array_header_in_bytes();
   int shift = value_array_klass->log2_element_size();
@@ -1637,6 +1639,18 @@
   }
 }
 
+void LIRGenerator::maybe_deopt_value_array_access(LIRItem& array, CodeEmitInfo* null_check_info, CodeEmitInfo* deopt_info) {
+  LIR_Opr klass = new_register(T_METADATA);
+  __ move(new LIR_Address(array.result(), oopDesc::klass_offset_in_bytes(), T_ADDRESS), klass, null_check_info);
+  LIR_Opr layout = new_register(T_INT);
+  __ move(new LIR_Address(klass, in_bytes(Klass::layout_helper_offset()), T_INT), layout);
+  __ shift_right(layout, Klass::_lh_array_tag_shift, layout);
+  __ cmp(lir_cond_equal, layout, LIR_OprFact::intConst(Klass::_lh_array_tag_vt_value));
+
+  CodeStub* stub = new DeoptimizeStub(deopt_info, Deoptimization::Reason_unloaded, Deoptimization::Action_make_not_entrant);
+  __ branch(lir_cond_equal, T_ILLEGAL, stub);
+}
+
 void LIRGenerator::do_StoreIndexed(StoreIndexed* x) {
   assert(x->is_pinned(),"");
   bool is_flattened = x->array()->is_flattened_array();
@@ -1696,17 +1710,24 @@
   }
 
   if (is_flattened) {
-    index.load_item();
-    access_flattened_array(false, array, index, value);
-  } else {
-    DecoratorSet decorators = IN_HEAP | IS_ARRAY;
-    if (x->check_boolean()) {
-      decorators |= C1_MASK_BOOLEAN;
+    if (x->array()->declared_type()->is_loaded()) {
+      index.load_item();
+      access_flattened_array(false, array, index, value);
+      return;
+    } else {
+      // If the array is indeed flattened, deopt. Otherwise access it as a normal object array.
+      CodeEmitInfo* deopt_info = state_for(x, x->state_before());
+      maybe_deopt_value_array_access(array, null_check_info, deopt_info);
     }
-
-    access_store_at(decorators, x->elt_type(), array, index.result(), value.result(),
-                    NULL, null_check_info);
   }
+
+  DecoratorSet decorators = IN_HEAP | IS_ARRAY;
+  if (x->check_boolean()) {
+    decorators |= C1_MASK_BOOLEAN;
+  }
+
+  access_store_at(decorators, x->elt_type(), array, index.result(), value.result(),
+                  NULL, null_check_info);
 }
 
 void LIRGenerator::access_load_at(DecoratorSet decorators, BasicType type,
@@ -1968,19 +1989,25 @@
   }
 
   if (x->array()->is_flattened_array()) {
-    // Find the destination address (of the NewValueTypeInstance)
-    LIR_Opr obj = x->vt()->operand();
-    LIRItem obj_item(x->vt(), this);
-
-    access_flattened_array(true, array, index, obj_item);
-    set_no_result(x);
-  } else {
-    DecoratorSet decorators = IN_HEAP | IS_ARRAY;
-    LIR_Opr result = rlock_result(x, x->elt_type());
-    access_load_at(decorators, x->elt_type(),
-                   array, index.result(), result,
-                   NULL, null_check_info);
+    if (x->array()->declared_type()->is_loaded()) {
+      // Find the destination address (of the NewValueTypeInstance)
+      LIR_Opr obj = x->vt()->operand();
+      LIRItem obj_item(x->vt(), this);
+
+      access_flattened_array(true, array, index, obj_item);
+      set_no_result(x);
+      return;
+    } else {
+      // If the array is indeed flattened, deopt. Otherwise access it as a normal object array.
+      CodeEmitInfo* deopt_info = state_for(x, x->state_before());
+      maybe_deopt_value_array_access(array, null_check_info, deopt_info);
+    }
   }
+  DecoratorSet decorators = IN_HEAP | IS_ARRAY;
+  LIR_Opr result = rlock_result(x, x->elt_type());
+  access_load_at(decorators, x->elt_type(),
+                 array, index.result(), result,
+                 NULL, null_check_info);
 }
 
 
--- a/src/hotspot/share/c1/c1_LIRGenerator.hpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/c1/c1_LIRGenerator.hpp	Tue Dec 11 15:26:06 2018 -0800
@@ -267,6 +267,7 @@
   void do_vectorizedMismatch(Intrinsic* x);
 
   void access_flattened_array(bool is_load, LIRItem& array, LIRItem& index, LIRItem& obj_item);
+  void maybe_deopt_value_array_access(LIRItem& array, CodeEmitInfo* null_check_info, CodeEmitInfo* deopt_info);
 
  public:
   LIR_Opr call_runtime(BasicTypeArray* signature, LIRItemList* args, address entry, ValueType* result_type, CodeEmitInfo* info);
--- a/src/hotspot/share/ci/ciEnv.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciEnv.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -394,7 +394,8 @@
 ciKlass* ciEnv::get_klass_by_name_impl(ciKlass* accessing_klass,
                                        const constantPoolHandle& cpool,
                                        ciSymbol* name,
-                                       bool require_local) {
+                                       bool require_local,
+                                       bool is_value_type) {
   ASSERT_IN_VM;
   EXCEPTION_CONTEXT;
 
@@ -408,7 +409,8 @@
                     sym->utf8_length()-2,
                     KILL_COMPILE_ON_FATAL_(_unloaded_ciinstance_klass));
     ciSymbol* strippedname = get_symbol(strippedsym);
-    return get_klass_by_name_impl(accessing_klass, cpool, strippedname, require_local);
+    is_value_type = (sym->char_at(0) == 'Q');
+    return get_klass_by_name_impl(accessing_klass, cpool, strippedname, require_local, is_value_type);
   }
 
   // Check for prior unloaded klass.  The SystemDictionary's answers
@@ -463,11 +465,12 @@
                                                  KILL_COMPILE_ON_FATAL_(fail_type));
 
     // Get element ciKlass recursively.
+    is_value_type = (sym->char_at(1) == 'Q');
     ciKlass* elem_klass =
       get_klass_by_name_impl(accessing_klass,
                              cpool,
                              get_symbol(elem_sym),
-                             require_local);
+                             require_local, is_value_type);
     if (elem_klass != NULL && elem_klass->is_loaded()) {
       // Now make an array for it
       if (elem_klass->is_valuetype() && elem_klass->as_value_klass()->flatten_array()) {
@@ -505,17 +508,23 @@
     i++;
   }
   if (i > 0 && sym->char_at(i) == 'Q') {
-    // An unloaded array class of value types is an ObjArrayKlass, an
-    // unloaded value type class is an InstanceKlass. For consistency,
-    // make the signature of the unloaded array of value type use L
-    // rather than Q.
-    char *new_name = CURRENT_THREAD_ENV->name_buffer(sym->utf8_length()+1);
-    strncpy(new_name, (char*)sym->base(), sym->utf8_length());
-    new_name[i] = 'L';
-    new_name[sym->utf8_length()] = '\0';
-    return get_unloaded_klass(accessing_klass, ciSymbol::make(new_name));
+    if (EnableValhallaC1) {
+      return get_unloaded_klass(accessing_klass, name, true);
+    } else {
+      // FIXME - C2 can't handle unloaded ciValueKlass
+
+      // An unloaded array class of value types is an ObjArrayKlass, an
+      // unloaded value type class is an InstanceKlass. For consistency,
+      // make the signature of the unloaded array of value type use L
+      // rather than Q.
+      char *new_name = CURRENT_THREAD_ENV->name_buffer(sym->utf8_length()+1);
+      strncpy(new_name, (char*)sym->base(), sym->utf8_length());
+      new_name[i] = 'L';
+      new_name[sym->utf8_length()] = '\0';
+      return get_unloaded_klass(accessing_klass, ciSymbol::make(new_name), false);
+    }
   }
-  return get_unloaded_klass(accessing_klass, name);
+  return get_unloaded_klass(accessing_klass, name, is_value_type);
 }
 
 // ------------------------------------------------------------------
--- a/src/hotspot/share/ci/ciEnv.hpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciEnv.hpp	Tue Dec 11 15:26:06 2018 -0800
@@ -139,7 +139,8 @@
   ciKlass* get_klass_by_name_impl(ciKlass* accessing_klass,
                                   const constantPoolHandle& cpool,
                                   ciSymbol* klass_name,
-                                  bool require_local);
+                                  bool require_local,
+                                  bool is_value_type = false);
   ciKlass*   get_klass_by_index_impl(const constantPoolHandle& cpool,
                                      int klass_index,
                                      bool& is_accessible,
@@ -243,8 +244,8 @@
   // Get a ciKlass representing an unloaded klass.
   // Ensures uniqueness of the result.
   ciKlass* get_unloaded_klass(ciKlass*  accessing_klass,
-                              ciSymbol* name) {
-    return _factory->get_unloaded_klass(accessing_klass, name, true);
+                              ciSymbol* name, bool is_value_type = false) {
+    return _factory->get_unloaded_klass(accessing_klass, name, true, is_value_type);
   }
 
   // Get a ciKlass representing an unloaded klass mirror.
--- a/src/hotspot/share/ci/ciObjectFactory.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciObjectFactory.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -450,7 +450,8 @@
 // unloaded klass.  This may need to change.
 ciKlass* ciObjectFactory::get_unloaded_klass(ciKlass* accessing_klass,
                                              ciSymbol* name,
-                                             bool create_if_not_found) {
+                                             bool create_if_not_found,
+                                             bool is_value_type) {
   EXCEPTION_CONTEXT;
   oop loader = NULL;
   oop domain = NULL;
@@ -504,7 +505,12 @@
       // The element klass is a TypeArrayKlass.
       element_klass = ciTypeArrayKlass::make(element_type);
     }
-    new_klass = new (arena()) ciObjArrayKlass(name, element_klass, dimension);
+    if (element_type == T_VALUETYPE && EnableValhallaC1) { // FIXME C2 can't handle unloaded ciValueArrayKlass
+      assert(element_klass->is_valuetype(), "must be");
+      new_klass = new (arena()) ciValueArrayKlass(name, element_klass->as_value_klass(), dimension);
+    } else {
+      new_klass = new (arena()) ciObjArrayKlass(name, element_klass, dimension);
+    }
   } else {
     jobject loader_handle = NULL;
     jobject domain_handle = NULL;
@@ -512,7 +518,11 @@
       loader_handle = accessing_klass->loader_handle();
       domain_handle = accessing_klass->protection_domain_handle();
     }
-    new_klass = new (arena()) ciInstanceKlass(name, loader_handle, domain_handle);
+    if (is_value_type && EnableValhallaC1) { // FIXME C2 can't handle unloaded ciValueKlass
+      new_klass = new (arena()) ciValueKlass(name, loader_handle, domain_handle);
+    } else {
+      new_klass = new (arena()) ciInstanceKlass(name, loader_handle, domain_handle);
+    }
   }
   init_ident_of(new_klass);
   _unloaded_klasses->append(new_klass);
--- a/src/hotspot/share/ci/ciObjectFactory.hpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciObjectFactory.hpp	Tue Dec 11 15:26:06 2018 -0800
@@ -114,7 +114,8 @@
   // Get a ciKlass representing an unloaded klass.
   ciKlass* get_unloaded_klass(ciKlass* accessing_klass,
                               ciSymbol* name,
-                              bool create_if_not_found);
+                              bool create_if_not_found,
+                              bool is_value_type = false);
 
   // Get a ciInstance representing an unresolved klass mirror.
   ciInstance* get_unloaded_klass_mirror(ciKlass* type);
--- a/src/hotspot/share/ci/ciValueArrayKlass.cpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciValueArrayKlass.cpp	Tue Dec 11 15:26:06 2018 -0800
@@ -55,6 +55,14 @@
   }
 }
 
+ciValueArrayKlass::ciValueArrayKlass(ciSymbol* array_name,
+                                     ciValueKlass* base_element_klass,
+                                     int dimension)
+  : ciArrayKlass(array_name, dimension, T_VALUETYPE) {
+  _base_element_klass = base_element_klass;
+  _element_klass = base_element_klass;
+}
+
 // ------------------------------------------------------------------
 // ciValueArrayKlass::element_klass
 //
@@ -148,9 +156,16 @@
     return CURRENT_THREAD_ENV->get_value_array_klass(array);
   }
 
-  // TODO handle this
-  guarantee(false, "klass not loaded");
-  return NULL;
+  // The array klass was unable to be made or the element klass was
+  // not loaded.
+  ciSymbol* array_name = construct_array_name(element_klass->name(), 1);
+  if (array_name == ciEnv::unloaded_cisymbol()) {
+    //return ciEnv::unloaded_ciobjarrayklass();
+    assert(0, "FIXME");
+  }
+  return
+    CURRENT_ENV->get_unloaded_klass(element_klass, array_name, false)
+                        ->as_value_array_klass();
 }
 
 // ------------------------------------------------------------------
--- a/src/hotspot/share/ci/ciValueArrayKlass.hpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciValueArrayKlass.hpp	Tue Dec 11 15:26:06 2018 -0800
@@ -42,6 +42,9 @@
 
 protected:
   ciValueArrayKlass(Klass* h_k);
+  ciValueArrayKlass(ciSymbol* array_name,
+                    ciValueKlass* element_klass,
+                    int dimension);
 
   ValueArrayKlass* get_ValueArrayKlass() {
     return (ValueArrayKlass*)get_Klass();
--- a/src/hotspot/share/ci/ciValueKlass.hpp	Wed Dec 12 10:08:39 2018 -0800
+++ b/src/hotspot/share/ci/ciValueKlass.hpp	Tue Dec 11 15:26:06 2018 -0800
@@ -47,6 +47,9 @@
     assert(is_final(), "ValueKlass must be final");
   };
 
+  ciValueKlass(ciSymbol* name, jobject loader, jobject protection_domain) :
+    ciInstanceKlass(name, loader, protection_domain) {}
+
   int compute_nonstatic_fields();
   const char* type_string() { return "ciValueKlass"; }
 
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnloadedValueTypeArray.java	Wed Dec 12 10:08:39 2018 -0800
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestUnloadedValueTypeArray.java	Tue Dec 11 15:26:06 2018 -0800
@@ -23,12 +23,19 @@
 
 /**
  * @test
- * @bug 8182997
- * @summary Array of unloaded value class should consistently use L signature
- * @compile -XDemitQtypes -XDenableValueTypes -XDallowFlattenabilityModifiers TestUnloadedValueTypeArray.java
- * @run main/othervm -XX:+EnableValhalla -Xcomp -XX:CompileOnly=TestUnloadedValueTypeArray::test TestUnloadedValueTypeArray
+ * @bug 8182997 8214898
+ * @library /test/lib
+ * @summary Test the handling of Arrays of unloaded value classes.
+ * @compile -XDemitQtypes -XDenableValueTypes -XDallowFlattenabilityModifiers -XDallowWithFieldOperator TestUnloadedValueTypeArray.java
+ * @run main/othervm -XX:+EnableValhalla -Xcomp
+ *        -XX:CompileCommand=compileonly,TestUnloadedValueTypeArray::test1
+ *        -XX:CompileCommand=compileonly,TestUnloadedValueTypeArray::test2
+ *        -XX:CompileCommand=compileonly,TestUnloadedValueTypeArray::test3
+ *      TestUnloadedValueTypeArray
  */
 
+import jdk.test.lib.Asserts;
+
 value final class MyValue {
     final int foo;
 
@@ -37,17 +44,89 @@
     }
 }
 
+value final class MyValue2 {
+    final int foo;
+
+    private MyValue2() {
+        foo = 0x42;
+    }
+    static MyValue2 make(int n) {
+        return __WithField(MyValue2.default.foo, n);
+    }
+}
+
+value final class MyValue3 {
+    final int foo;
+
+    private MyValue3() {
+        foo = 0x42;
+    }
+    static MyValue3 make(int n) {
+        return __WithField(MyValue3.default.foo, n);
+    }
+}
+
+
+
 public class TestUnloadedValueTypeArray {
 
     static MyValue[] target() {
         return new MyValue[10];
     }
 
-    static void test() {
+    static void test1() {
         target();
     }
 
+    static int test2(MyValue2[] arr) {
+        if (arr != null) {
+            return arr[1].foo;
+        } else {
+            return 1234;
+        }
+    }
+
+    static void test2_verifier() {
+        int n = 50000;
+
+        int m = 9999;
+        for (int i=0; i<n; i++) {
+            m = test2(null);
+        }
+        Asserts.assertEQ(m, 1234);
+
+        MyValue2[] arr = new MyValue2[2];
+        arr[1] = MyValue2.make(5678);
+        m = 9999;
+        for (int i=0; i<n; i++) {
+            m = test2(arr);
+        }
+        Asserts.assertEQ(m, 5678);
+    }
+
+    static void test3(MyValue3[] arr) {
+        if (arr != null) {
+            arr[1] = MyValue3.make(2345);
+        }
+    }
+
+    static void test3_verifier() {
+        int n = 50000;
+
+        for (int i=0; i<n; i++) {
+            test3(null);
+        }
+
+        MyValue3[] arr = new MyValue3[2];
+        for (int i=0; i<n; i++) {
+            test3(arr);
+        }
+        Asserts.assertEQ(arr[1].foo, 2345);
+    }
+
     static public void main(String[] args) {
-        test();
+        test1();
+        test2_verifier();
+        test3_verifier();
     }
 }