changeset 53163:3bafd37b0e58 lworld

8214894: [lworld] Allocation elimination should not be blocked by ValueTypeNodes
author thartmann
date Wed, 05 Dec 2018 18:01:40 +0100
parents 7a2b07f41171
children 69425f4cef12
files src/hotspot/share/ci/ciTypeFlow.cpp src/hotspot/share/opto/macro.cpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java
diffstat 4 files changed, 91 insertions(+), 89 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/ci/ciTypeFlow.cpp	Tue Dec 04 11:36:08 2018 -0800
+++ b/src/hotspot/share/ci/ciTypeFlow.cpp	Wed Dec 05 18:01:40 2018 +0100
@@ -273,12 +273,6 @@
 ciType* ciTypeFlow::StateVector::type_meet_internal(ciType* t1, ciType* t2, ciTypeFlow* analyzer) {
   assert(t1 != t2, "checked in caller");
 
-  // Unwrap the types after gathering nullness information
-  bool never_null1 = t1->is_never_null();
-  bool never_null2 = t2->is_never_null();
-  t1 = t1->unwrap();
-  t2 = t2->unwrap();
-
   if (t1->equals(top_type())) {
     return t2;
   } else if (t2->equals(top_type())) {
@@ -299,60 +293,66 @@
     // At least one of the two types is a non-top primitive type.
     // The other type is not equal to it.  Fall to bottom.
     return bottom_type();
+  }
+
+  // Unwrap the types after gathering nullness information
+  bool never_null1 = t1->is_never_null();
+  bool never_null2 = t2->is_never_null();
+  t1 = t1->unwrap();
+  t2 = t2->unwrap();
+
+  // Both types are non-top non-primitive types.  That is,
+  // both types are either instanceKlasses or arrayKlasses.
+  ciKlass* object_klass = analyzer->env()->Object_klass();
+  ciKlass* k1 = t1->as_klass();
+  ciKlass* k2 = t2->as_klass();
+  if (k1->equals(object_klass) || k2->equals(object_klass)) {
+    return object_klass;
+  } else if (!k1->is_loaded() || !k2->is_loaded()) {
+    // Unloaded classes fall to java.lang.Object at a merge.
+    return object_klass;
+  } else if (k1->is_interface() != k2->is_interface()) {
+    // When an interface meets a non-interface, we get Object;
+    // This is what the verifier does.
+    return object_klass;
+  } else if (k1->is_array_klass() || k2->is_array_klass()) {
+    // When an array meets a non-array, we get Object.
+    // When objArray meets typeArray, we also get Object.
+    // And when typeArray meets different typeArray, we again get Object.
+    // But when objArray meets objArray, we look carefully at element types.
+    if (k1->is_obj_array_klass() && k2->is_obj_array_klass()) {
+      // Meet the element types, then construct the corresponding array type.
+      ciKlass* elem1 = k1->as_obj_array_klass()->element_klass();
+      ciKlass* elem2 = k2->as_obj_array_klass()->element_klass();
+      ciKlass* elem  = type_meet_internal(elem1, elem2, analyzer)->as_klass();
+      // Do an easy shortcut if one type is a super of the other.
+      if (elem == elem1) {
+        assert(k1 == ciObjArrayKlass::make(elem), "shortcut is OK");
+        return k1;
+      } else if (elem == elem2) {
+        assert(k2 == ciObjArrayKlass::make(elem), "shortcut is OK");
+        return k2;
+      } else {
+        return ciObjArrayKlass::make(elem);
+      }
+    } else if (k1->is_value_array_klass() || k2->is_value_array_klass()) {
+      ciKlass* elem1 = k1->as_array_klass()->element_klass();
+      ciKlass* elem2 = k2->as_array_klass()->element_klass();
+      ciKlass* elem  = type_meet_internal(elem1, elem2, analyzer)->as_klass();
+      return ciArrayKlass::make(elem);
+    } else {
+      return object_klass;
+    }
   } else {
-    // Both types are non-top non-primitive types.  That is,
-    // both types are either instanceKlasses or arrayKlasses.
-    ciKlass* object_klass = analyzer->env()->Object_klass();
-    ciKlass* k1 = t1->as_klass();
-    ciKlass* k2 = t2->as_klass();
-    if (k1->equals(object_klass) || k2->equals(object_klass)) {
-      return object_klass;
-    } else if (!k1->is_loaded() || !k2->is_loaded()) {
-      // Unloaded classes fall to java.lang.Object at a merge.
-      return object_klass;
-    } else if (k1->is_interface() != k2->is_interface()) {
-      // When an interface meets a non-interface, we get Object;
-      // This is what the verifier does.
-      return object_klass;
-    } else if (k1->is_array_klass() || k2->is_array_klass()) {
-      // When an array meets a non-array, we get Object.
-      // When objArray meets typeArray, we also get Object.
-      // And when typeArray meets different typeArray, we again get Object.
-      // But when objArray meets objArray, we look carefully at element types.
-      if (k1->is_obj_array_klass() && k2->is_obj_array_klass()) {
-        // Meet the element types, then construct the corresponding array type.
-        ciKlass* elem1 = k1->as_obj_array_klass()->element_klass();
-        ciKlass* elem2 = k2->as_obj_array_klass()->element_klass();
-        ciKlass* elem  = type_meet_internal(elem1, elem2, analyzer)->as_klass();
-        // Do an easy shortcut if one type is a super of the other.
-        if (elem == elem1) {
-          assert(k1 == ciObjArrayKlass::make(elem), "shortcut is OK");
-          return k1;
-        } else if (elem == elem2) {
-          assert(k2 == ciObjArrayKlass::make(elem), "shortcut is OK");
-          return k2;
-        } else {
-          return ciObjArrayKlass::make(elem);
-        }
-      } else if (k1->is_value_array_klass() || k2->is_value_array_klass()) {
-        ciKlass* elem1 = k1->as_array_klass()->element_klass();
-        ciKlass* elem2 = k2->as_array_klass()->element_klass();
-        ciKlass* elem  = type_meet_internal(elem1, elem2, analyzer)->as_klass();
-        return ciArrayKlass::make(elem);
-      } else {
-        return object_klass;
-      }
-    } else {
-      // Must be two plain old instance klasses.
-      assert(k1->is_instance_klass(), "previous cases handle non-instances");
-      assert(k2->is_instance_klass(), "previous cases handle non-instances");
-      ciType* result = k1->least_common_ancestor(k2);
-      if (never_null1 && never_null2 && result->is_valuetype()) {
-        // Both value types are never null, mark the result as never null
-        result = analyzer->mark_as_never_null(result);
-      }
-      return result;
+    // Must be two plain old instance klasses.
+    assert(k1->is_instance_klass(), "previous cases handle non-instances");
+    assert(k2->is_instance_klass(), "previous cases handle non-instances");
+    ciType* result = k1->least_common_ancestor(k2);
+    if (never_null1 && never_null2 && result->is_valuetype()) {
+      // Both value types are never null, mark the result as never null
+      result = analyzer->mark_as_never_null(result);
     }
+    return result;
   }
 }
 
@@ -630,6 +630,7 @@
   } else {
     pop_object();
     if (str->get_never_null()) {
+      // Casting to a Q-Type contains a NULL check
       assert(klass->is_valuetype(), "must be a value type");
       push(outer()->mark_as_never_null(klass));
     } else {
@@ -773,7 +774,7 @@
     outer()->record_failure("ldc did not link");
     return;
   }
-  if (basic_type == T_OBJECT || basic_type == T_OBJECT || basic_type == T_ARRAY) {
+  if (basic_type == T_OBJECT || basic_type == T_VALUETYPE || basic_type == T_ARRAY) {
     ciObject* obj = con.as_object();
     if (obj->is_null_object()) {
       push_null();
--- a/src/hotspot/share/opto/macro.cpp	Tue Dec 04 11:36:08 2018 -0800
+++ b/src/hotspot/share/opto/macro.cpp	Wed Dec 05 18:01:40 2018 +0100
@@ -710,6 +710,8 @@
         } else {
           safepoints.append_if_missing(sfpt);
         }
+      } else if (use->is_ValueType() && use->isa_ValueType()->get_oop() == res) {
+        // ok to eliminate
       } else if (use->Opcode() != Op_CastP2X) { // CastP2X is used by card mark
         if (use->is_Phi()) {
           if (use->outcnt() == 1 && use->unique_out()->Opcode() == Op_Return) {
@@ -1044,6 +1046,10 @@
         }
 
         _igvn._worklist.push(ac);
+      } else if (use->is_ValueType()) {
+        assert(use->isa_ValueType()->get_oop() == res, "unexpected value type use");
+         _igvn.rehash_node_delayed(use);
+        use->isa_ValueType()->set_oop(_igvn.zerocon(T_VALUETYPE));
       } else {
         eliminate_gc_barrier(use);
       }
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java	Tue Dec 04 11:36:08 2018 -0800
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java	Wed Dec 05 18:01:40 2018 +0100
@@ -166,9 +166,8 @@
     }
 
     // Merge value types created from two branches
-// TODO fix this once we can distinguish between nullable and non-nullable value types
-//    @Test(failOn = ALLOC + STORE + TRAP)
-    @Test()
+    @Test(valid = AlwaysIncrementalInlineOn, match = {ALLOC, STORE, LOAD}, matchCount = {1, 5, 12}, failOn = TRAP)
+    @Test(valid = AlwaysIncrementalInlineOff, failOn = ALLOC + STORE + TRAP)
     public long test8(boolean b) {
         MyValue1 v;
         if (b) {
@@ -629,8 +628,8 @@
     }
 
     // Verify that C2 recognizes value type loads and re-uses the oop to avoid allocations
+    @Test(valid = ValueTypeReturnedAsFieldsOn)
     @Test(valid = ValueTypeReturnedAsFieldsOff, failOn = ALLOC + ALLOCA + STORE)
-    @Test(valid = ValueTypeReturnedAsFieldsOn)
     public MyValue3 test31(MyValue3[] va) {
         // C2 can re-use the oop returned by createDontInline()
         // because the corresponding value type is equal to 'copy'.
@@ -650,8 +649,8 @@
     }
 
     // Verify that C2 recognizes value type loads and re-uses the oop to avoid allocations
+    @Test(valid = ValueTypePassFieldsAsArgsOn)
     @Test(valid = ValueTypePassFieldsAsArgsOff, failOn = ALLOC + ALLOCA + STORE)
-    @Test(valid = ValueTypePassFieldsAsArgsOn)
     public MyValue3 test32(MyValue3 vt, MyValue3[] va) {
         // C2 can re-use the oop of vt because vt is equal to 'copy'.
         MyValue3 copy = MyValue3.copy(vt);
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java	Tue Dec 04 11:36:08 2018 -0800
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java	Wed Dec 05 18:01:40 2018 +0100
@@ -92,9 +92,10 @@
     public static final long rL = Utils.getRandomInstance().nextLong() % 1000;
 
     // User defined settings
+    protected static final boolean XCOMP = Platform.isComp();
     private static final boolean PRINT_GRAPH = true;
     private static final boolean PRINT_TIMES = Boolean.parseBoolean(System.getProperty("PrintTimes", "false"));
-    private static       boolean VERIFY_IR = Boolean.parseBoolean(System.getProperty("VerifyIR", "true")) && (!TEST_C1);
+    private static       boolean VERIFY_IR = Boolean.parseBoolean(System.getProperty("VerifyIR", "true")) && !TEST_C1 && !XCOMP;
     private static final boolean VERIFY_VM = Boolean.parseBoolean(System.getProperty("VerifyVM", "false"));
     private static final String SCENARIOS = System.getProperty("Scenarios", "");
     private static final String TESTLIST = System.getProperty("Testlist", "");
@@ -105,12 +106,13 @@
     // Pre-defined settings
     private static final List<String> defaultFlags = Arrays.asList(
         "-XX:-BackgroundCompilation", "-XX:CICompilerCount=1",
-        "-XX:+PrintCompilation", "-XX:+PrintIdeal", "-XX:+PrintOptoAssembly",
         "-XX:CompileCommand=quiet",
         "-XX:CompileCommand=compileonly,java.lang.invoke.*::*",
         "-XX:CompileCommand=compileonly,java.lang.Long::sum",
         "-XX:CompileCommand=compileonly,java.lang.Object::<init>",
         "-XX:CompileCommand=compileonly,compiler.valhalla.valuetypes.*::*");
+    private static final List<String> printFlags = Arrays.asList(
+        "-XX:+PrintCompilation", "-XX:+PrintIdeal", "-XX:+PrintOptoAssembly");
     private static final List<String> verifyFlags = Arrays.asList(
         "-XX:+VerifyOops", "-XX:+VerifyStack", "-XX:+VerifyLastFrame", "-XX:+VerifyBeforeGC", "-XX:+VerifyAfterGC",
         "-XX:+VerifyDuringGC", "-XX:+VerifyAdapterSharing", "-XX:+StressValueTypeReturnedAsFields");
@@ -122,16 +124,18 @@
     protected static final int ValueTypeArrayFlattenOff = 0x8;
     protected static final int ValueTypeReturnedAsFieldsOn = 0x10;
     protected static final int ValueTypeReturnedAsFieldsOff = 0x20;
+    protected static final int AlwaysIncrementalInlineOn = 0x40;
+    protected static final int AlwaysIncrementalInlineOff = 0x80;
     static final int AllFlags = ValueTypePassFieldsAsArgsOn | ValueTypePassFieldsAsArgsOff | ValueTypeArrayFlattenOn | ValueTypeArrayFlattenOff | ValueTypeReturnedAsFieldsOn;
     protected static final boolean ValueTypePassFieldsAsArgs = (Boolean)WHITE_BOX.getVMFlag("ValueTypePassFieldsAsArgs");
     protected static final boolean ValueTypeArrayFlatten = (Boolean)WHITE_BOX.getVMFlag("ValueArrayFlatten");
     protected static final boolean ValueTypeReturnedAsFields = (Boolean)WHITE_BOX.getVMFlag("ValueTypeReturnedAsFields");
+    protected static final boolean AlwaysIncrementalInline = (Boolean)WHITE_BOX.getVMFlag("AlwaysIncrementalInline");
     protected static final int COMP_LEVEL_ANY = -2;
     protected static final int COMP_LEVEL_FULL_OPTIMIZATION = TEST_C1 ? 1 : 4;
     protected static final Hashtable<String, Method> tests = new Hashtable<String, Method>();
     protected static final boolean USE_COMPILER = WHITE_BOX.getBooleanVMFlag("UseCompiler");
     protected static final boolean PRINT_IDEAL  = WHITE_BOX.getBooleanVMFlag("PrintIdeal");
-    protected static final boolean XCOMP = Platform.isComp();
 
     // Regular expressions used to match nodes in the PrintIdeal output
     protected static final String START = "(\\d+\\t(.*";
@@ -309,29 +313,15 @@
         Asserts.assertFalse(tests.isEmpty(), "no tests to execute");
         ArrayList<String> args = new ArrayList<String>(defaultFlags);
         String[] vmInputArgs = InputArguments.getVmInputArgs();
+        for (String arg : vmInputArgs) {
+            if (arg.startsWith("-XX:CompileThreshold")) {
+                // Disable IR verification if non-default CompileThreshold is set
+                VERIFY_IR = false;
+            }
+        }
         if (VERIFY_IR) {
-            for (String arg : vmInputArgs) {
-                if (arg.startsWith("-XX:CompileThreshold")) {
-                    // Disable IR verification if
-                    VERIFY_IR = false;
-                }
-                // Check if the JVM supports value type specific default arguments from the test's run commands
-                if (arg.startsWith("-XX:+ValueTypePassFieldsAsArgs") ||
-                    arg.startsWith("-XX:+ValueTypeReturnedAsFields")) {
-                    Boolean value = (Boolean)WHITE_BOX.getVMFlag(arg.substring(5));
-                    if (!value) {
-                        System.out.println("WARNING: could not enable " + arg.substring(5) + ". Skipping IR verification.");
-                        VERIFY_IR = false;
-                    }
-                } else if (arg.startsWith("-XX:-ValueTypePassFieldsAsArgs") ||
-                           arg.startsWith("-XX:-ValueTypeReturnedAsFields")) {
-                    Boolean value = (Boolean)WHITE_BOX.getVMFlag(arg.substring(5));
-                    if (value) {
-                        System.out.println("WARNING: could not disable " + arg.substring(5) + ". Skipping IR verification.");
-                        VERIFY_IR = false;
-                    }
-                }
-            }
+            // Add print flags for IR verification
+            args.addAll(printFlags);
             // Always trap for exception throwing to not confuse IR verification
             args.add("-XX:-OmitStackTraceInFastThrow");
         }
@@ -417,6 +407,12 @@
                 } else if ((a.valid() & ValueTypeReturnedAsFieldsOff) != 0 && !ValueTypeReturnedAsFields) {
                     assert anno == null;
                     anno = a;
+                } else if ((a.valid() & AlwaysIncrementalInlineOn) != 0 && AlwaysIncrementalInline) {
+                    assert anno == null;
+                    anno = a;
+                } else if ((a.valid() & AlwaysIncrementalInlineOff) != 0 && !AlwaysIncrementalInline) {
+                    assert anno == null;
+                    anno = a;
                 }
             }
             assert anno != null;