RFR(S): 8217990: C2 UseOptoBiasInlining: load of markword optimized to 0 if running with -XX:-EliminateLocks

[Reingruber, Richard]

Hi Roland,

thanks for taking the bug and providing a fix, which looks good to me. I'm not a reviewer, though, but I tested it and had a look at the generated ideal graph.

I noticed that the markword load is still pinned. Is this necessary?

I guess so, because without pinning it could possibly be scheduled before a safepoint as it loads from a known instance which does not interfere memory wise with the safepoint. Or in other words: if the safepoint is a call, the load does not depend on the memory state produced by the call, because it refers to a known instance which is not passed as parameter.

Thanks again, as well for adding the copyright header,
Richard.

-----Original Message-----
From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Doerr, Martin
Sent: Donnerstag, 7. Februar 2019 15:32
To: Tobias Hartmann <tobias.hartmann at oracle.com>; Roland Westrelin <rwestrel at redhat.com>; hotspot-compiler-dev at openjdk.java.net
Subject: [CAUTION] RE: RFR(S): 8217990: C2 UseOptoBiasInlining: load of markword optimized to 0 if running with -XX:-EliminateLocks

Hi Roland,

looks good to me, too. Also thanks for adding the copyright.

Best regards,
Martin


-----Original Message-----
From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Tobias Hartmann
Sent: Donnerstag, 7. Februar 2019 14:53
To: Roland Westrelin <rwestrel at redhat.com>; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(S): 8217990: C2 UseOptoBiasInlining: load of markword optimized to 0 if running with -XX:-EliminateLocks

Hi Roland,

this looks good to me, thanks for fixing.

Best regards,
Tobias

On 07.02.19 10:44, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/8217990/webrev.00/
> 
> As correctly explained in the bug report, the load of the mark word
> created at macro expansion time for newly allocated objects has wrong
> memory state.
> 
> It's true for non escaping allocation because
> GraphKit::set_output_for_allocation) doesn't add edges for header fields
> to the MergeMem that captures the memory state after a new
> allocation. It's also armless AFAICT but still should better be fixed as
> proposed in the bug report.
> 
> For escaping allocations, the load of the mark work can be optimized to
> 0 as stated in the bug report. The root cause is that when EA runs,
> there's no load/store for the mark word in the graph. So
> ConnectionGraph::split_unique_types() doesn't update the memory graph
> with a new alias for the mark word of a non escaping allocation. The fix
> I propose is to always allocate an alias for the mark word of non
> escaping allocations. Then EA takes it into account when updating
> MergeMem nodes in the graph.
> 
> I also did the same for loads of the klass pointer.
> 
> The test case was provided by SAP without a copyright header. I copy
> pasted a SAP copyright header that I found in another test case. I hope
> it's ok.
> 
> Roland.
>