NullCheckOp

[Deneau, Tom]

Thomas --

To overcome the problem mentioned below, I made the following one line change in GuardLoweringPhase.
However, I am wondering if there should be a target-specific boolean indicating whether
a target could support implicitNullChecks rather than just a single global option.
That way, if you are compiling for two different targets (say cpu and gpu), one could
use implicit null checks and one could use explicit.

-- Tom D.

     3.1 --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/GuardLoweringPhase.java	Tue Dec 24 16:03:03 2013 -0600
     3.2 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/GuardLoweringPhase.java	Mon Dec 30 08:03:01 2013 -0600
     3.3 @@ -143,7 +143,7 @@
     3.4          protected void processNode(Node node) {
     3.5              if (node instanceof GuardNode) {
     3.6                  GuardNode guard = (GuardNode) node;
     3.7 -                FixedWithNextNode lowered = guard.lowerGuard();
     3.8 +                FixedWithNextNode lowered = (OptImplicitNullChecks.getValue() ? guard.lowerGuard() : null);
     3.9                  if (lowered != null) {
    3.10                      replaceCurrent(lowered);
    3.11                  } else {
    3.12                      lowerToIf(guard);
    3.13                  }

> -----Original Message-----
> From: Deneau, Tom
> Sent: Saturday, December 28, 2013 9:00 AM
> To: 'Thomas Wuerthinger'
> Cc: graal-dev at openjdk.java.net; Christian Thalinger
> Subject: RE: NullCheckOp
> 
> Thomas --
> 
> I tried -G:-OptImplicitNullChecks, it did not make any difference.
> I think the problem is here:
> 
>     private static void processBlock(Block block, SchedulePhase
> schedule, int implicitNullCheckLimit) {
>         if (OptImplicitNullChecks.getValue() && implicitNullCheckLimit >
> 0) {
>             new
> UseImplicitNullChecks(implicitNullCheckLimit).processNodes(block,
> schedule);
>         }
>         new LowerGuards(block).processNodes(block, schedule);
>     }
> 
> In that we still go thru LowerGuards.processNodes which eventually gets
> to GuardNode.lowerGuard and the if block conditions are all met here so
> the NullCheckNode is created...
> 
>     public FixedWithNextNode lowerGuard() {
>         if (negated() && condition() instanceof IsNullNode) {
>             IsNullNode isNull = (IsNullNode) condition();
>             NullCheckNode nullCheck = graph().add(new
> NullCheckNode(isNull.object()));
>             setCondition(null);
>             if (isNull.usages().isEmpty()) {
>                 isNull.safeDelete();
>             }
>             return nullCheck;
>         }
> 
>         return null;
>     }
> 
> I have confirmed that if I force GuardNode.lowerGuard to just return
> null, the explicit null comparison and deopt node works correctly.
> 
> -- Tom
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Wuerthinger [mailto:thomas.wuerthinger at oracle.com]
> > Sent: Saturday, December 28, 2013 6:41 AM
> > To: Deneau, Tom
> > Cc: graal-dev at openjdk.java.net; Christian Thalinger
> > Subject: Re: NullCheckOp
> >
> > Tom,
> >
> > The flag "OptImplicitNullChecks" controls whether the compiler tries
> > to create implicit null check instructions. If you set it to false,
> > the code generated is equivalent to a comparison of the value against
> > null and a deoptimization if the comparison yields true.
> >
> > - thomas
> >
> > On 24 Dec 2013, at 03:45, Christian Thalinger
> > <christian.thalinger at oracle.com> wrote:
> >
> > > I'm not an expert on this but it seems that a NullCheckOp (or a
> > NullCheckNode) only does an implicit null check; there is no branch
> > information attached to it.
> > >
> > > In order to do an explicit exception you'd need to add nodes to the
> > graph that throw the exception.  Not sure if something like this
> > exists already.
> > >
> > > On Dec 23, 2013, at 1:24 PM, Deneau, Tom <tom.deneau at amd.com> wrote:
> > >
> > >> I am trying to get null checks working on the hsail backend.
> > >> What is required of NullCheckOp in the HSAILLIRGenerator?
> > >> On Hsail, we can't support an implicit exception for this like
> > >> amd64
> > does  so I think we would just want to end up with an explicit compare
> > and branch if null to something that would be similar to the code
> > emitted for a deoptimizing node.  How much of that does NullCheckOp
> > have to do?
> > >>
> > >> -- Tom
> > >
> >