RFR: 8256406: G1 x86 C1/Interpreter post write barrier always uses 32 bit to access variable sized PtrQueue::_index

[Thomas Schatzl]

On Tue, 17 Nov 2020 17:19:44 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this change that fixes benign issues with the G1 C1/interpreter barriers that on x64 only used the lower 32 bit word of the 64 bit PtrQueue::_index member?
>> 
>> Testing: tier1, linux-x86 builds
>> 
>> Thanks,
>>   Thomas
>
> src/hotspot/share/gc/shared/ptrQueue.hpp line 181:
> 
>> 179:   }
>> 180: 
>> 181:   static constexpr ByteSize byte_width_of_index() { return in_ByteSize(sizeof(size_t)); }
> 
> Do we really have to `constexpr` this for `STATIC_ASSERT`? No objection, just curious.

My compiler (gcc 9.3.0) complains with

`[...] g1BarrierSetAssembler_x86.cpp:269:60: error: call to non-'constexpr' function 'static ByteSize PtrQueue::byte_width_of_index()' [...]`

otherwise. Probably because of the called `in_ByteSize()` method.

> src/hotspot/cpu/x86/gc/g1/g1BarrierSetAssembler_x86.cpp line 322:
> 
>> 320:   __ movptr(tmp2, queue_index);
>> 321:   __ testptr(tmp2, tmp2);
>> 322:   __ jcc(Assembler::equal, runtime);
> 
> I know it is the same in x86-speak, but `Assembler::zero` would be more readable to show that we actually test `queue_index == 0`.

You mean using `__ cmpptr(tmp2, 0)` or something similar? I would like to keep this that way as it's such a common and well-known idiom, even if it's only a nano-optimization.

Otherwise we should also change the pre-barrier code to be uniform.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1257