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

[dean.long at oracle.com]

Looks OK, but could you remind me why PatchALot uses O7 in mem2reg/reg2mem?

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