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

Bengt Rutisson bengt.rutisson at oracle.com
Fri May 22 07:30:45 UTC 2015

On 21/05/15 15:57, Roland Westrelin wrote:
> Hi Bengt,
>> 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
>> 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.
> That looks good to me. Please add a comment.

Thanks Roland. I'll add this comment before I push:

      // Use unsigned type T_BOOLEAN here rather than signed T_BYTE 
since some platforms, eg. ARM,
      // need to use unsigned instructions to use the large offset to 
load the satb_mark_queue.
      flag_type = T_BOOLEAN;


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

More information about the hotspot-dev mailing list