Request for review(S): 6991512: G1 barriers fail with 64bit C1

[Igor Veresov]

Thanks, Vladimir and Tom!

igor


On 10/12/10 9:25 PM, Vladimir Kozlov wrote:
> Got it. Looks good.
>
> Thanks,
> Vladimir
>
> On 10/12/10 8:32 PM, Igor Veresov wrote:
>> On 10/12/10 7:51 PM, Vladimir Kozlov wrote:
>>> Why 18 and not 16?:
>>
>> bpr instruction uses an simm16 offset in words. I guess I chose a bad
>> name for the method. I renamed it to
>> is_in_wdisp16_range() to reflect that fact better. Webrev updated
>> in-place.
>>
>> igor
>>
>>>
>>> + return is_simm(d, 18);
>>>
>>> Other changes are good.
>>>
>>> Vladimir
>>>
>>> On 10/12/10 7:30 PM, Igor Veresov wrote:
>>>> On 10/12/10 5:41 PM, Tom Rodriguez wrote:
>>>>>
>>>>> On Oct 12, 2010, at 5:12 PM, Igor Veresov wrote:
>>>>>
>>>>>> This fixes two problems:
>>>>>> 1. On x64, c1_LIR.hpp:401,
>>>>>> # assert(is_single_cpu()&& !is_virtual()) failed: type check
>>>>>>
>>>>>> Which is due to passing a pointer to T_LONG instead of T_OBJECT to
>>>>>> G1 post-barrier when emitting LIR for
>>>>>> CompareAndSwap intrinsic. The fix is to create a register with an
>>>>>> appropriate type when CASing an object reference.
>>>>>
>>>>> Looks good.
>>>>>
>>>>>>
>>>>>> 2. On sparc, assembler_sparc.hpp:843,
>>>>>> # assert(nbits == 32 || -(1<< nbits-1)<= x&& x< ( 1<< nbits-1))
>>>>>> failed: value out of range
>>>>>>
>>>>>> This one is due to the limited jump range of branch-on-reg-value
>>>>>> instruction. The solution is to use cmp+brx since
>>>>>> the jump target is not guaranteed to be in 16 bit range (the target
>>>>>> is a continuation of a stub).
>>>>>
>>>>> In this particular case the label should be bound when emit_code is
>>>>> called so you could in fact check the distance and
>>>>> use br_on_reg_cond if it will reach. It seems like it mostly reaches.
>>>>> Using the longer sequence is fine too.
>>>>>
>>>>
>>>> Yes, it does in most of the cases...
>>>>
>>>> Here is the updated webrev:
>>>> http://cr.openjdk.java.net/~iveresov/6991512/webrev.01/
>>>>
>>>> igor
>>>>
>>>>> tom
>>>>>
>>>>>>
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/6991512/webrev.00/
>>>>>>
>>>>>>
>>>>>> Tested with a couple of failed nightlies.
>>>>>>
>>>>>> Thanks,
>>>>>> igor
>>>>>>
>>>>>
>>>>
>>