Bug ID: JDK-8054492 Casting can result in redundant null checks in generated code

JDK 9
9 b40Fixed

Adding intrinsic for Class.cast() may solve JDK-8035809 problem too.
27-10-2014
Next small change removed NULL check from compiled code for Class.cast: diff -r f111958ca117 src/share/vm/classfile/vmSymbols.hpp --- a/src/share/vm/classfile/vmSymbols.hpp Fri Sep 19 17:14:13 2014 +0200 +++ b/src/share/vm/classfile/vmSymbols.hpp Thu Oct 09 11:26:18 2014 -0700 @@ -455,6 +455,7 @@ template(object_void_signature, "(Ljava/lang/Object;)V") \ template(object_int_signature, "(Ljava/lang/Object;)I") \ template(object_boolean_signature, "(Ljava/lang/Object;)Z") \ + template(object_object_signature, "(Ljava/lang/Object;)Ljava/lang/Object;") \ template(string_void_signature, "(Ljava/lang/String;)V") \ template(string_int_signature, "(Ljava/lang/String;)I") \ template(throwable_void_signature, "(Ljava/lang/Throwable;)V") \ @@ -746,6 +747,8 @@ do_name( isPrimitive_name, "isPrimitive") \ do_intrinsic(_getSuperclass, java_lang_Class, getSuperclass_name, void_class_signature, F_RN) \ do_name( getSuperclass_name, "getSuperclass") \ + do_intrinsic(_class_cast, java_lang_Class, class_cast_name, object_object_signature, F_R) \ + do_name( class_cast_name, "cast") \ \ do_intrinsic(_getClassAccessFlags, sun_reflect_Reflection, getClassAccessFlags_name, class_int_signature, F_SN) \ do_name( getClassAccessFlags_name, "getClassAccessFlags") \ diff -r f111958ca117 src/share/vm/opto/parse2.cpp --- a/src/share/vm/opto/parse2.cpp Fri Sep 19 17:14:13 2014 +0200 +++ b/src/share/vm/opto/parse2.cpp Thu Oct 09 11:26:18 2014 -0700 @@ -933,6 +933,9 @@ float cnt; float prob = branch_prediction(cnt, btest, target_bci); + if (method()->intrinsic_id() == vmIntrinsics::_class_cast) { + prob = PROB_FAIR; + } if (prob == PROB_UNKNOWN) { // (An earlier version of do_ifnull omitted this trap for OSR methods.) #ifndef PRODUCT
09-10-2014
I talked with Paul and he is asking to not generate uncommon trap for null check in Class.cast() in all cases - just the branch to merge point. If isInstance code is statically folded (very common case) then null check will be eliminate (as in second case above). Currently the null check is remaining even if isInstance code is folded. One benefit from having uncommon trap branch with null check is it could be converted to implicit null check by folding it into a following memory instruction (load or store through checked object pointer). In Class.cast() we don't have such memory instruction so we don't benefit from uncommon trap. We can do that by intrinsifying Class.cast(), for example. Or by checking compiled method when we generate null check.
09-10-2014
I think what's going on is this: in the first case we find the if (obj != null) { else branch to be never taken from profiling and aggressively compile it as if (obj == null) uncommon_trap; (which usually pays off) then the isInstance and cast itself go away because the compiler can statistically prove they are fine. In the second case, profiling reports both branches of the if (obj != null) to be taken. The isInstance and cast are still proven to always succeed so go away. So the code is: if (obj != null) { return obj; } else { return obj; } and the compiler can remove the if() control flow because it's essentially empy. If this is to help the type profiler and not actually useful code, wouldn't argument profiling that we've added a few months ago be good enough? Do you really need to help the type profiler?
26-08-2014
In this simple example it seems the compiler removes everything for the cast except the null check? The generated code for the following: for (int i = 0; i < 1000000; i++) { testLoopOne(svalue); testLoopOne(svalue); } 0x0000000105d0c640: mov 0xc(%rbp),%r10d ;getfield svalue ; - NullCheckDroppingsTest::testLoop@10 (line 16) 0x0000000105d0c644: test %r10d,%r10d 0x0000000105d0c647: je 0x0000000105d0c689 ;ifnull ; - java.lang.Class::cast@1 (line 3257) ; - NullCheckDroppingsTest::testLoopOne@4 (line 38) ; - NullCheckDroppingsTest::testLoop@13 (line 16) 0x0000000105d0c649: mov %r10d,0x14(%rbp) ;putfield ssink ; - NullCheckDroppingsTest::testLoopOne@10 (line 38) ; - NullCheckDroppingsTest::testLoop@13 (line 16) 0x0000000105d0c64d: mov 0xc(%rbp),%r10d 0x0000000105d0c651: mov %rbp,%r8 0x0000000105d0c654: shr $0x9,%r8 0x0000000105d0c658: mov %r12b,(%r11,%r8,1) ;getfield svalue ; - NullCheckDroppingsTest::testLoop@18 (line 19) 0x0000000105d0c65c: test %r10d,%r10d 0x0000000105d0c65f: je 0x0000000105d0c6a1 ;ifnull ; - java.lang.Class::cast@1 (line 3257) ; - NullCheckDroppingsTest::testLoopOne@4 (line 38) ; - NullCheckDroppingsTest::testLoop@21 (line 19) 0x0000000105d0c661: mov %r10d,0x14(%rbp) ;goto ; - NullCheckDroppingsTest::testLoop@27 (line 15) 0x0000000105d0c665: mov %rbp,%r10 0x0000000105d0c668: shr $0x9,%r10 0x0000000105d0c66c: mov %r12b,(%r11,%r10,1) ;putfield ssink ; - NullCheckDroppingsTest::testLoopOne@10 (line 38) ; - NullCheckDroppingsTest::testLoop@21 (line 19) 0x0000000105d0c670: inc %ebx ;iinc ; - NullCheckDroppingsTest::testLoop@24 (line 15) 0x0000000105d0c672: cmp $0xf4240,%ebx 0x0000000105d0c678: jl 0x0000000105d0c640 ;if_icmpge ; - NullCheckDroppingsTest::testLoop@5 (line 15) And for: for (int i = 0; i < 1000000; i++) { testLoopOne(svalue); testLoopOne(snull); } 0x000000011060d400: mov 0xc(%rbp),%r11d ;getfield svalue ; - NullCheckDroppingsTest::testLoop@10 (line 16) 0x000000011060d404: mov %r11d,0x14(%rbp) ;putfield ssink ; - NullCheckDroppingsTest::testLoopOne@10 (line 38) ; - NullCheckDroppingsTest::testLoop@13 (line 16) 0x000000011060d408: mov 0x10(%rbp),%r11d 0x000000011060d40c: mov %rbp,%r8 0x000000011060d40f: shr $0x9,%r8 0x000000011060d413: mov %r12b,(%r10,%r8,1) ;getfield snull ; - NullCheckDroppingsTest::testLoop@18 (line 19) 0x000000011060d417: mov %r11d,0x14(%rbp) ;goto ; - NullCheckDroppingsTest::testLoop@27 (line 15) 0x000000011060d41b: mov %rbp,%r11 0x000000011060d41e: shr $0x9,%r11 0x000000011060d422: mov %r12b,(%r10,%r11,1) ;putfield ssink ; - NullCheckDroppingsTest::testLoopOne@10 (line 38) ; - NullCheckDroppingsTest::testLoop@21 (line 19) 0x000000011060d426: inc %ebx ;iinc ; - NullCheckDroppingsTest::testLoop@24 (line 15) 0x000000011060d428: cmp $0xf4240,%ebx 0x000000011060d42e: jl 0x000000011060d400 ;if_icmpge ; - NullCheckDroppingsTest::testLoop@5 (line 15) And for: for (int i = 0; i < 1000000; i++) { testLoopOne(snull); testLoopOne(snull); } 0x00000001098a1831: mov 0x10(%rbx),%r10d ;getfield snull ; - NullCheckDroppingsTest::testLoop@10 (line 16) 0x00000001098a1835: test %r10d,%r10d 0x00000001098a1838: jne 0x00000001098a1869 ;ifnull ; - java.lang.Class::cast@1 (line 3257) ; - NullCheckDroppingsTest::testLoopOne@4 (line 38) ; - NullCheckDroppingsTest::testLoop@13 (line 16) 0x00000001098a183a: mov %r12d,0x14(%rbx) ;invokevirtual testLoopOne ; - NullCheckDroppingsTest::testLoop@13 (line 16) 0x00000001098a183e: mov 0x10(%rbx),%r11d ;getfield snull ; - NullCheckDroppingsTest::testLoop@18 (line 19) 0x00000001098a1842: test %r11d,%r11d 0x00000001098a1845: jne 0x00000001098a1885 ;ifnull ; - java.lang.Class::cast@1 (line 3257) ; - NullCheckDroppingsTest::testLoopOne@4 (line 38) ; - NullCheckDroppingsTest::testLoop@21 (line 19) 0x00000001098a1847: mov %r12d,0x14(%rbx) ;goto ; - NullCheckDroppingsTest::testLoop@27 (line 15) 0x00000001098a184b: inc %ebp ;iinc ; - NullCheckDroppingsTest::testLoop@24 (line 15) 0x00000001098a184d: cmp $0xf4240,%ebp 0x00000001098a1853: jl 0x00000001098a1831 ;if_icmpge ; - NullCheckDroppingsTest::testLoop@5 (line 15) (Options used "-XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -XX:LoopMaxUnroll=1 -XX:+PrintAssembly") FYI the context here is direct MHs to a (non-Object) ref field use a cast to say "Hey type profiler take notice!".
26-08-2014
Class.cast has an explicit null check: public T cast(Object obj) { if (obj != null && !isInstance(obj)) throw new ClassCastException(cannotCastMsg(obj)); return (T) obj; } It's the one triggering the deoptimization. Given if affects the result of cast I don't see how it can be removed.
26-08-2014
The following test shows deoptimization behaviour due to the redundant null check and if certain code is uncommented how the redundant null check goes away if null and non-null values are observed. public class NullCheckDroppingsTest { volatile String svalue = "A"; volatile String snull = null; String ssink; public static void main(String[] args) { NullCheckDroppingsTest t = new NullCheckDroppingsTest(); t.testLoop(); } void testLoop() { // Ensure testLoopOne is compiled and does not // see a null value for (int i = 0; i < 1000000; i++) { testLoopOne(svalue); // Uncomment the following and the call outside // the loop should not result in any deoptimization // testLoopOne(snull); } // Sleep just to create delay in the compilation log try { Thread.currentThread().sleep(1000); } catch (Exception e) {} // This should cause a de-optimisation // if testLoopOne is compiled with a null-check testLoopOne(snull); } void testLoopOne(String s) { try { ssink = String.class.cast(s); } catch (Throwable t) { throw new Error(t); } } } $ java -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -XX:+PrintCompilation NullCheckDroppingsTest 64 1 java.lang.String::hashCode (55 bytes) 69 2 java.lang.String::indexOf (70 bytes) 81 3 ! NullCheckDroppingsTest::testLoopOne (27 bytes) 81 5 n java.lang.Class::isInstance (native) 81 4 java.lang.Class::cast (27 bytes) 82 6 % ! NullCheckDroppingsTest::testLoop @ 2 (45 bytes) 85 6 % ! NullCheckDroppingsTest::testLoop @ -2 (45 bytes) made not entrant 1086 3 ! NullCheckDroppingsTest::testLoopOne (27 bytes) made not entrant
07-08-2014

Blocks :	JDK-8062543 - Replace uses of MethodHandleImpl.castReference with Class.cast
Relates :	JDK-8035809 - Improve inlining of Class::cast method
Relates :	JDK-8062477 - Add speculative type profiling for parameters of Class.cast() and Class.isInstance() methods