NullCheckOp
Gilles Duboscq
duboscq at ssw.jku.at
Wed Jan 15 02:48:42 PST 2014
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/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