NullCheckOp
Deneau, Tom
tom.deneau at amd.com
Wed Jan 15 07:06:27 PST 2014
OK, we will merge with that...
-- Tom
> -----Original Message-----
> From: gilwooden at gmail.com [mailto:gilwooden at gmail.com] On Behalf Of
> Gilles Duboscq
> Sent: Wednesday, January 15, 2014 4:49 AM
> To: Deneau, Tom
> Cc: Thomas Wuerthinger; graal-dev at openjdk.java.net
> Subject: Re: NullCheckOp
>
> Tom,
>
> I separated the code that introduce those trapping null checks to a
> separate phase [1] which should respect implicitNullCheckLimit=0.
>
> -Gilles
>
> [1] http://hg.openjdk.java.net/graal/graal/rev/e57115c41164
>
> On Wed, Jan 8, 2014 at 11:28 PM, Tom Deneau <tom.deneau at amd.com> wrote:
> > Thomas --
> >
> > Sorry, I never answered back on this one.
> > I am in favor of just using the already existing
> implicitNullCheckLimit=0 as a disabler.
> >
> > I have made the following change locally here and it works fine for
> our backend...
> >
> > diff -r 9cd47b39b0ef
> graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/
> GuardLoweringPhase.java
> > ---
> a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/commo
> n/GuardLoweringPhase.java Tue Jan 07 16:32:58 2014 -0800
> > +++
> b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/commo
> n/GuardLoweringPhase.java Wed Jan 08 16:25:28 2014 -0600
> > @@ -126,17 +126,19 @@
> >
> > private final Block block;
> > private boolean useGuardIdAsSpeculationId;
> > + private boolean shouldUseImplicitNullChecks;
> >
> > - public LowerGuards(Block block, boolean
> useGuardIdAsSpeculationId) {
> > + public LowerGuards(Block block, boolean
> > + useGuardIdAsSpeculationId, boolean shouldUseImplicitNullChecks) {
> > this.block = block;
> > this.useGuardIdAsSpeculationId =
> > useGuardIdAsSpeculationId;
> > + this.shouldUseImplicitNullChecks =
> > + shouldUseImplicitNullChecks;
> > }
> >
> > @Override
> > protected void processNode(Node node) {
> > if (node instanceof GuardNode) {
> > GuardNode guard = (GuardNode) node;
> > - FixedWithNextNode lowered = guard.lowerGuard();
> > + FixedWithNextNode lowered =
> > + (shouldUseImplicitNullChecks ? guard.lowerGuard() : null);
> > if (lowered != null) {
> > replaceCurrent(lowered);
> > } else {
> > @@ -193,9 +195,10 @@
> > }
> >
> > private static void processBlock(Block block, SchedulePhase
> schedule, int implicitNullCheckLimit) {
> > - if (OptImplicitNullChecks.getValue() &&
> implicitNullCheckLimit > 0) {
> > + boolean shouldUseImplicitNullChecks =
> OptImplicitNullChecks.getValue() && implicitNullCheckLimit > 0;
> > + if (shouldUseImplicitNullChecks) {
> > new
> UseImplicitNullChecks(implicitNullCheckLimit).processNodes(block,
> schedule);
> > }
> > - new LowerGuards(block, Debug.isDumpEnabledForMethod() ||
> Debug.isLogEnabledForMethod()).processNodes(block, schedule);
> > + new LowerGuards(block, Debug.isDumpEnabledForMethod() ||
> > + Debug.isLogEnabledForMethod(),
> > + shouldUseImplicitNullChecks).processNodes(block, schedule);
> > }
> > }
> >
> > -- Tom
> >
> >
> >> -----Original Message-----
> >> From: Thomas Wuerthinger [mailto:thomas.wuerthinger at oracle.com]
> >> Sent: Friday, January 03, 2014 8:26 AM
> >> To: Deneau, Tom
> >> Cc: graal-dev at openjdk.java.net; Christian Thalinger
> >> Subject: Re: NullCheckOp
> >>
> >> Tom,
> >>
> >> We can either introduce a new field in the TargetDescription class or
> >> disable all implicit null checks when
> >> TargetDescription.implicitNullCheckLimit equals 0.
> >>
> >> - thomas
> >>
> >> On 02 Jan 2014, at 23:16, Deneau, Tom <tom.deneau at amd.com> wrote:
> >>
> >> > 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/commo
> >> n/GuardLoweringPhase.java Tue Dec 24 16:03:03 2013 -0600
> >> > 3.2 +++
> >>
> b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/commo
> >> n/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
> >> >>>>
> >> >>>
> >> >
> >> >
> >>
> >
> >
More information about the graal-dev
mailing list