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

[Vladimir Kozlov]

Paul,

There was an other place (speculative types code) which added the NULL 
check. Add next changes in parse2.cpp to previous patch:

@@ -2223,6 +2243,7 @@
      a = null();
      b = pop();
      if (!_gvn.type(b)->speculative_maybe_null() &&
+        (method()->intrinsic_id() != vmIntrinsics::_class_cast) &&
          !too_many_traps(Deoptimization::Reason_speculate_null_check)) {
        inc_sp(1);
        Node* null_ctl = top();

Whole patch:

http://cr.openjdk.java.net/~kvn/8054492/webrev/

Thanks,
Vladimir


On 10/14/14 9:58 AM, Paul Sandoz wrote:
> Here is a simpler test case that just uses a MH to Class.cast that exhibits the same behaviour.
>
> The commented out use of the BiFunction created from a lambda expressions works fine.
>
> This suggests there might be something about the MH invocation that is stopping the optimization from kicking in.
>
> Paul.
>
> import java.lang.invoke.MethodHandle;
> import java.lang.invoke.MethodHandles;
> import java.lang.invoke.MethodType;
> import java.util.function.BiFunction;
>
> public class NullCheckDroppingsMHCastTest {
>
>      static final MethodHandle MH_CAST;
>
>      static {
>          try {
>              MH_CAST = MethodHandles.lookup().findVirtual(Class.class,
>                                                           "cast",
>                                                           MethodType.methodType(Object.class, Object.class));
>          }
>          catch (Exception e) {
>              throw new Error(e);
>          }
>      }
>
>      static final BiFunction<Class, Object, Object> fCast = (c, o) -> c.cast(o);
>
>
>      volatile String svalue = "A";
>      volatile String snull = null;
>      String ssink;
>
>      public static void main(String[] args) {
>          NullCheckDroppingsMHCastTest t = new NullCheckDroppingsMHCastTest();
>          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) (Object) MH_CAST.invokeExact(String.class, (Object) s);
> //            ssink = (String) fCast.apply(String.class, s);
>          }
>          catch (Throwable t) {
>              throw new Error(t);
>          }
>      }
> }
>