RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI (off-list)

Fri Dec 14 09:02:16 UTC 2018

Hi Igor,

thank you for the review. Pushed (jdk13).

Best regards,
Martin

From: Igor Veresov <igor.veresov at oracle.com>
Sent: Donnerstag, 13. Dezember 2018 17:17
To: Doerr, Martin <martin.doerr at sap.com>
Cc: hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI (off-list)

Ok, I think this makes sense. Looks good.

igor

On Dec 13, 2018, at 2:59 AM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:

Hi Igor,

I understand that there are more important things to work on at the moment. Thanks for taking some time for reviewing my proposal.

With JVMTI “can_access_local_variables”, should_retain_local_variables() is true which keeps locals alive (ciMethod::liveness_at_bci) until they get reused (or a conflicting type).
(For debugging, locals need to get kept alive within their scopes, but scope information may not be available, so C1 needs to be conservative enough.)
So “can_access_local_variables” causes the “for_each_local_value” iterator to find locals which would otherwise have been considered dead out after their last usage.

In my schematic example, C1 calls setup_phi_for_local for the object in line 803 in recent c1_Instruction.cpp when coming from Block 1. When coming from Block 2, C1 reaches line 832 because “existing_state->is_same(new_state)”. The code after line 847 is not used.

Best regards,
Martin

From: Igor Veresov <igor.veresov at oracle.com<mailto:igor.veresov at oracle.com>>
Sent: Donnerstag, 13. Dezember 2018 00:03
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI (off-list)

Martin,

Sorry I’ve got distracted from this review. I’m trying to understand why the phi is created in the first place. There is a code just below your change in c1_Instruction.cpp that should invalidate the local. Have you found out why doesn’t it work?

igor

On Dec 10, 2018, at 5:24 AM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:

Hi Igor,

just let me know if you need more explanations. I know, the change is small, but not trivial.
I can target this enhancement to jdk13 if you don’t have time at the moment even though it is tested very well.

The real life bailouts seem to be hard to analyze, but they can be observed often:
jdk/bin/java -XX:+PrintBailouts -agentlib:jdwp=transport=dt_socket,address=8000,server=y,suspend=n -Djava.io.tmpdir=results -jar OctaneLauncher.jar

starting runNashorn.js
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
Richards: 95.9
DeltaBlue: 71.9
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
Crypto: 315
compilation bailout: block join failed
RayTrace: 320
EarleyBoyer: 652
compilation bailout: too many stack slots used
RegExp: 268
compilation bailout: too many stack slots used
Splay: 356
SplayLatency: 228
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
NavierStokes: 450
compilation bailout: CodeBuffer overflow
compilation bailout: CodeBuffer overflow
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: CodeBuffer overflow
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
compilation bailout: block join failed
… (also many other bailout reasons)

The “compilation bailout: block join failed” completely disappear with my change, so I still think this change is worth the effort.

Best regards,
Martin

From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net<mailto:hotspot-compiler-dev-bounces at openjdk.java.net>> On Behalf Of Doerr, Martin
Sent: Dienstag, 4. Dezember 2018 18:46
To: Igor Veresov <igor.veresov at oracle.com<mailto:igor.veresov at oracle.com>>
Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>>; hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: [CAUTION] RE: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI

Hi Igor,

thank you for looking at my proposal.
Let me try to explain it by a schematic example.

Block 1
local 1 used for some object (assignment and usage)

Block 2
local 1 used for an int (assignment and usage)
loop: if ... goto Block 2

C1 creates a Phi function (with input local 1 of type object) when looking at the edge from Block 1 to Block 2 (GraphBuilder::iterate_bytecodes_for_block => BlockBegin::try_merge => setup_phi_for_local).
When looking at the backedge (the loop in Block 2), BlockBegin::try_merge sees the "existing_value" of type object and the "new_value" of type int coming from the backedge.
This is the point at which we know that we are beyond the life ranges of local l. It's dead because a life local can't have conflicting types.

I think the appropriate treatment is to invalidate the local and the Phi function. Seems like an old version had already done this according to the comment "The old code invalidated the phi function here" probably because linear scan had issues and the problematic case was rare enough (without JVMTI). But now we have a bailout in LIRGenerator::move_to_phi which handles the really problematic case, so we can use the invalidation in BlockBegin::try_merge again which is far better when using JVMTI.

AFAIK, this matches what C2 would do.

Hope this makes it more clear. Let me know if I missed anything.

Thanks and best regards,
Martin

From: Igor Veresov <igor.veresov at oracle.com<mailto:igor.veresov at oracle.com>>
Sent: Dienstag, 4. Dezember 2018 18:13
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com<mailto:vladimir.kozlov at oracle.com>>; hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI

So how and why does it happen that a phi has different input types?

igor

On Dec 3, 2018, at 2:24 AM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:

Hi Vladimir,

thanks for reviewing. I hope a C1 expert can find some time to take a look.

Best regards,
Martin

-----Original Message-----
From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net<mailto:hotspot-compiler-dev-bounces at openjdk.java.net>> On Behalf Of Vladimir Kozlov
Sent: Sonntag, 2. Dezember 2018 01:50
To: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>
Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI

Seems fine to me but I am not C1 expert.

Regards,
Vladimir

On 11/27/18 3:03 AM, Doerr, Martin wrote:
Hi,

C1 currently bails out when a local value is derived from a Phi with different types.

According to comments, this case is supposed to be very rare, but it's not so rare when a JVMTI agent is used
(capability "can_access_local_variables").

I'd like to fix this.

Issue with more detailed description:

https://bugs.openjdk.java.net/browse/JDK-8214352

Webrev:

http://cr.openjdk.java.net/~mdoerr/8214352_c1_phi/webrev.00/
<http://cr.openjdk.java.net/%7Emdoerr/8214352_c1_phi/webrev.00/>

Patch successfully ran through a substantial amount of tests with enabled "can_access_local_variables". Please review.

Best regards,

Martin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181214/e533b03e/attachment-0001.html>