RFR: JDK-8080627: JavaThread::satb_mark_queue_offset() is too big for an ARM ldrsb instruction

[Bengt Rutisson]

Hi Kim,

On 21/05/15 23:55, Kim Barrett wrote:
> On May 20, 2015, at 4:15 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>>
>> Hi everyone,
>>
>> This is a fix for the C1 generated G1 write barriers. It is a bit unclear if this is compiler or GC code, so I'm using the broader mailing list for this review.
>>
>> http://cr.openjdk.java.net/~brutisso/8080627/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8080627
> Using a Java type designator for a C++ value seems fishy, but not unique to here, so oh well.

I totally agree.

>
> Looks good.

Thanks for the review!

Bengt

>
>> The problem is that on ARM the T_BYTE type will boil down to using the ldrsb instruction, which has a limitation on the offset it can load from. It can only load from offsets -256 to 256. But in the G1 pre barrier we want to load the _satb_mark_queue field in JavaThread, which is on offset 760.
>>
>> Changing the type from T_BYTE to T_BOOLEAN will use the unsigned instruction ldrb instead, which can handle offsets up to 4096. Ideally we would have a T_UBYTE type to use unsigned instructions for this load, but that does not exist.
>>
>> On the other platforms (x86 and Sparc) we treat T_BYTE and T_BOOLEAN the same, it is only on ARM that we have the distinction between these to types. I assume that is to get the sign extension for free when we use T_BYTE type. The fact that we treat T_BYTE and T_BOOLEAN the same on the other platforms makes it safe to do this change.
>>
>> I got some great help with this change from Dean Long. Thanks, Dean!
>>
>> I tried a couple of different solutions. Moving the _satb_mark_queue field earlier in JavaThread did not help since the Thread superclass already has enough members to exceed the 256 limit for offsets. It also didn't seem like a stable solution. Loading the field into a register would work, but keeping the load an immediate seems like a nicer solution. Changing to treat T_BYTE and T_BOOLEAN the same on ARM (similarly to x86 and Sparc) would mean to have to do explicit sign extension, which seems like a more complex solution than just switching the type in this case.
>>
>> Bengt
>