RFR (XS): 8023472: C2 optimization breaks with G1
Azeem Jiva
azeem.jiva at oracle.com
Thu Aug 22 06:48:32 PDT 2013
Adding the GC dev alias to get their opinion.
--
Azeem Jiva
@javawithjiva
On Aug 22, 2013, at 6:34 AM, "Doerr, Martin" <martin.doerr at sap.com> wrote:
> Hi,
>
> we have an addon to your G1 pre-barrier change which reduces
> its cost.
> It is possible to get rid of the do_load in inline_unsafe_load_store
> because we always know the value which gets overwritten.
>
> In case of cmpxchg, the only value which might get overwritten
> is known in advance.
> In case of xchg, the old value is returned by the load_store.
> It is allowed to do the barrier after the xchg because there's
> no safepoint between them.
>
> Please review
> http://cr.openjdk.java.net/~goetz/webrevs/opto_g1_barr/
>
> Best regards,
> Martin and Goetz
>
>
> -----Original Message-----
> From: hotspot-compiler-dev-bounces at openjdk.java.net [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
> Sent: Donnerstag, 22. August 2013 02:55
> To: hotspot compiler
> Subject: Re: RFR (XS): 8023472: C2 optimization breaks with G1
>
> Thanks, Christian
>
> On 8/21/13 4:04 PM, Christian Thalinger wrote:
>>
>> On Aug 21, 2013, at 3:18 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>>> http://cr.openjdk.java.net/~kvn/8023472/webrev/
>>>
>>> The load of previous value in G1 pre-barrier code (method GraphKit::g1_write_barrier_pre()) does not have control. It is from the time when G1 code was merged into Hotspot sources. I can only speculate that it was done to allow commoning with a preceding normal load if it exists and to avoid generation of 2 loads.
>>> This problem, I think, only affects loads from an array because a load from a field should be guarded by NULL check of object.
>>>
>>> The fix is to put control on this load to always generate it after if (marking) check. I doubt it affect performance much. If there was load before the value should be in cache.
>>
>> The fix looks good. Can you at least do a refworkload run to do some due diligence?
>
> Will do it tonight.
>
>>>
>>> Added regression test.
>>
>> Could you change the name of the test? Since we moved away from using bug ids the test name should more or less explain what it tests. I suppose there will be more G1CrashTest's in the future :-)
>
> It is external test. I don't want to change it much or its name.
>
> Vladimir
>
>>
>> -- Chris
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20130822/144d8f10/attachment.html
More information about the hotspot-compiler-dev
mailing list