--- old/src/hotspot/share/ci/ciTypeFlow.cpp 2018-12-05 15:54:25.350739019 +0100 +++ new/src/hotspot/share/ci/ciTypeFlow.cpp 2018-12-05 15:54:25.162735568 +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())) { @@ -295,64 +289,70 @@ return t1; } } - + // 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(); - } 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); + } + + // 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 object_klass; + 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 { - // 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; + 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; } } @@ -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(); --- old/src/hotspot/share/opto/macro.cpp 2018-12-05 15:54:25.798747245 +0100 +++ new/src/hotspot/share/opto/macro.cpp 2018-12-05 15:54:25.622744014 +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); } --- old/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java 2018-12-05 15:54:26.346757307 +0100 +++ new/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java 2018-12-05 15:54:26.138753487 +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 = ValueTypeReturnedAsFieldsOff, failOn = ALLOC + ALLOCA + STORE) @Test(valid = ValueTypeReturnedAsFieldsOn) + @Test(valid = ValueTypeReturnedAsFieldsOff, failOn = ALLOC + ALLOCA + STORE) 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 = ValueTypePassFieldsAsArgsOff, failOn = ALLOC + ALLOCA + STORE) @Test(valid = ValueTypePassFieldsAsArgsOn) + @Test(valid = ValueTypePassFieldsAsArgsOff, failOn = ALLOC + ALLOCA + STORE) 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); --- old/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java 2018-12-05 15:54:26.850766560 +0100 +++ new/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java 2018-12-05 15:54:26.642762741 +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 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::", "-XX:CompileCommand=compileonly,compiler.valhalla.valuetypes.*::*"); + private static final List printFlags = Arrays.asList( + "-XX:+PrintCompilation", "-XX:+PrintIdeal", "-XX:+PrintOptoAssembly"); private static final List 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 tests = new Hashtable(); 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 args = new ArrayList(defaultFlags); String[] vmInputArgs = InputArguments.getVmInputArgs(); - 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; - } - } + for (String arg : vmInputArgs) { + if (arg.startsWith("-XX:CompileThreshold")) { + // Disable IR verification if non-default CompileThreshold is set + VERIFY_IR = false; } + } + if (VERIFY_IR) { + // 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;