NullCheckOp
Deneau, Tom
tom.deneau at amd.com
Wed Jan 8 14:28:38 PST 2014
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/common/GuardLoweringPhase.java Tue Jan 07 16:32:58 2014 -0800
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/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