[9] RFR(S): 8171435: "assert(is_single_cpu() && !is_virtual()) failed: type check" with -XX:+PatchALot on SPARC

[Tobias Hartmann]

Hi Dean,

Thanks for looking at this!

On 06.01.2017 22:19, dean.long at oracle.com wrote:
> Looks OK, but could you remind me why PatchALot uses O7 in mem2reg/reg2mem?

LIR_Assembler::mem2reg() uses O7 to hold the result of the (src + index) computation in the unaligned case (or with PatchALot):

1303   } else if (unaligned || PatchALot) {
1304     __ add(src, addr->index()->as_pointer_register(), O7);
1305     src = O7;
[...]
1445     offset = store(from_reg, src, disp_value, type, wide, unaligned);

Best regards,
Tobias

> 
> dl
> 
> 
> On 1/5/17 9:23 AM, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8171435
>> http://cr.openjdk.java.net/~thartmann/8171435/webrev.00/
>>
>> On SPARC, -XX:+PatchALot is used to treat memory accesses as unaligned (in addition to treating field classes as unloaded). There are multiple problems if PatchALot is enabled or addresses are unaligned:
>>
>> 1) In LIR_Assembler::mem2reg/reg2mem() we add addr->index() to addr->base() and store the result in O7 if the address is unaligned or PatchALot is enabled. If a long value is loaded/stored, addr->index() resides in a double cpu register. For example, "L4L4" in the failing case:
>>
>>   move [Base:[L3|M] Index:[L4L4|J] Disp: 0|J] [L5L5|J]
>>
>> Using as_register() to get the addr->index() register hits an assert because !is_single_cpu(). We should use as_pointer_register() instead.
>>
>> 2) In LIR_Assembler::store() we use O7 to temporarily hold the lower 32 bits of an unaligned store to memory. However, in case of 1) we already use O7 in the caller method LIR_Assembler::reg2mem() to hold 'base'. Overwriting it results in a crash. LIR_Assembler::load() handles this correctly:
>>
>>   // in case O7 is base or offset, use it last
>>
>> I changed the code to use G3_scratch instead and also added additional asserts that would have caught this problem.
>>
>> 3) In LIR_Assembler::load(), we should treat the load as unaligned if PatchALot is enabled.
>>
>> I modified the CanonicalizeArrayLength.java test to trigger both crashes.
>>
>> Tested with failing test and RBT (running).
>>
>> Thanks,
>> Tobias
>