RFR: Fix conditional store in interpreter matrix barrier

[Roman Kennke]

Am 04.10.2017 um 19:50 schrieb Aleksey Shipilev:
> On 10/04/2017 07:44 PM, Roman Kennke wrote:
>> We attempt to do a conditional store in the interpreter matrix barrier. We use testb(addr, 0) for
>> this. testb and-s its operands and sets the condition flags. And-ing with 0 always yields 0, so the
>> zero-flag will always be set, resulting in the matrix barrier always storing 1 into the matrix. This
>> is not fatal, but it leads to unintentional false sharing and the like.
>>
>> Fix:
>>
>> diff --git a/src/cpu/x86/vm/macroAssembler_x86.cpp b/src/cpu/x86/vm/macroAssembler_x86.cpp
>> --- a/src/cpu/x86/vm/macroAssembler_x86.cpp
>> +++ b/src/cpu/x86/vm/macroAssembler_x86.cpp
>> @@ -5493,7 +5493,7 @@
>>     // Address is _matrix[to * stride + from]
>>     movptr(rscratch1, (intptr_t) matrix_addr);
>>     // Test if the element is already set.
>> -  testb(Address(rscratch1, tmp, Address::times_1), 0);
>> +  testb(Address(rscratch1, tmp, Address::times_1), 1);
>>     jcc(Assembler::notZero, done);
>>     // Store true, if not yet set.
>>     movb(Address(rscratch1, tmp, Address::times_1), 1);
>>
>> Test: hotspot_gc_shenandoah
>>
>> ok?
> Looks good. D'oh. Maybe we should use cmpb(..., 0) + jcc(notEqual, done) for clarity -- no strong
> opinion here though.
>
> -Aleksey
>
Ok, this is a good idea. I'm pushing this:

diff --git a/src/cpu/x86/vm/macroAssembler_x86.cpp 
b/src/cpu/x86/vm/macroAssembler_x86.cpp
--- a/src/cpu/x86/vm/macroAssembler_x86.cpp
+++ b/src/cpu/x86/vm/macroAssembler_x86.cpp
@@ -5493,8 +5493,8 @@
    // Address is _matrix[to * stride + from]
    movptr(rscratch1, (intptr_t) matrix_addr);
    // Test if the element is already set.
-  testb(Address(rscratch1, tmp, Address::times_1), 0);
-  jcc(Assembler::notZero, done);
+  cmpb(Address(rscratch1, tmp, Address::times_1), 0);
+  jcc(Assembler::notEqual, done);
    // Store true, if not yet set.
    movb(Address(rscratch1, tmp, Address::times_1), 1);
    bind(done);