RFR (XS): 8023472: C2 optimization breaks with G1

[Vladimir Kozlov]

On 8/21/13 5:55 PM, Vladimir Kozlov wrote:
> 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.

Finally found sparc to test (which should be most affected).

Vladimir

============================================================================
   Benchmark         Samples        Mean     Stdev   %Diff    P 
Significant
   scimark                20      303.63      1.23   -0.07 0.616 
     *
     LU                   20      608.89      5.44   -0.16 0.593 
     *
     FFT                  20       20.12      0.03    0.00 0.925 
     *
     Monte                20       99.26      0.12   -0.01 0.893 
     *
     SOR                  20      565.11      1.33   -0.00 0.981 
     *
     Sparse               20      224.79      0.56   -0.00 0.955 
     *
   scimark_small          20      448.46      4.54   -0.08 0.807 
     *
     LU                   20      838.92     22.93   -0.28 0.757 
     *
     FFT                  20      344.25      0.37    0.03 0.463 
     *
     Monte                20       98.89      0.48   -0.04 0.760 
     *
     SOR                  20      569.50      0.54    0.05 0.262 
     *
     Sparse               20      390.75      0.53    0.04 0.472 
     *
   specjvm98              20      279.43      1.03    0.06 0.732 
     *
     compress             20      284.47      4.66    0.35 0.512 
     *
     javac                20      188.70      1.23   -0.29 0.202 
     *
     db                   20       74.51      0.63    0.05 0.854 
     *
     jack                 20      188.29      2.93   -0.99 0.024 
     *
     mtrt                 20     1008.67     28.74    0.30 0.753 
     *
     jess                 20      308.44      2.25    0.98 0.047 
     *
     mpegaudio            20      568.12      2.36   -0.00 0.982 
     *
============================================================================

>
>>>
>>> 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
>>>