[9] RFR [XS] 8054492: Casting can result in redundant null checks in generated code

[John Rose]

On Oct 29, 2014, at 1:14 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> I hope it is the final version.
> 
> http://cr.openjdk.java.net/~kvn/8054492/webrev.02/
> 
> Full blown intrinsic is created for Class.cast(). It tries, first, to fold statically the code. If it can't, it calls gen_checkcast() and generate uncommon trap when the exception have to be thrown. Also if we hit uncommon trap too match it will not generate instrinsic with dynamic checks during recompilation, but it still tries to fold it statically.
> 
> Since the intrinsic can be skipped I marked Class.cast() as force_inline method to make sure it is inlined to propagate checkcast information in compiled code.

Good.  Suggest a comment like:
      set_intrinsic_id(id);
+     if (id == vmIntrinsics::_class_cast) {
++     // even if the intrinsic is rejected, we want to inline this simple method
+       set_force_inline(true);
+     }
      return;

Nit:  should be "_Class_cast" not "_class_cast" (cf. _Short_valueOf etc.).

+      // Don't use intrinsic, have to throw ClassCastException.
Suggest additional comment:
++     // If the reference is null, the non-intrinsic bytecode will optimize appropriately.

A similar point probably applies to the !is_loaded case which you handle next.
Suggest just bailing out of the intrinsic for that case, same as SSC_always_false.
That might simplify the code.

Good test, although I have a sneaking suspicion that nobody is testing corner cases like:
   int.class.cast(null);  // OK
   int.class.cast(42);  // CCE int != Integer
I suggest doing a little informal coverage analysis to make sure your new code branches get exercised.

It looks good; count me a reviewer.

> An additional improvement could be made to type checks in intrinsics if we use speculative types. I filed RFE:
> https://bugs.openjdk.java.net/browse/JDK-8062477

Thanks for filing that.
> 
> I added new regression test and added compile_id to WB nmethod information.
> 
> Tested jtreg hotspot and jtreg j/l/invoke from jdk, CTW, JPRT, performance.
> 
> Nashorn's Octane-Box2D and Octane-DeltaBlue shows few percents improvement.
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141029/fb0ce2a6/attachment-0001.html>