RFR(m): 8220351: Cross-modifying code

[Andrew Haley]

On 3/11/19 10:05 AM, Robbin Ehn wrote:
>> What is this hunk for?
>>
>> @@ -2558,12 +2558,9 @@
>>     //     VM thread changes sync state to synchronizing and suspends threads for GC.
>>     //     Thread A is resumed to finish this native method, but doesn't block here since it
>>     //     didn't see any synchronization is progress, and escapes.
>> -  __ movl(Address(r15_thread, JavaThread::thread_state_offset()), _thread_in_native_trans);
>> -
>> -  // Force this write out before the read below
>> -  __ membar(Assembler::Membar_mask_bits(
>> -              Assembler::LoadLoad | Assembler::LoadStore |
>> -              Assembler::StoreLoad | Assembler::StoreStore));
>> +
>> +  __ lock();
>> +  __ addl(Address(r15_thread, JavaThread::thread_state_offset()), 1 /* _thread_in_native_trans */);
>>
>>     Label after_transition;
>>
> 
> Yes, a bit unrelated.
> I thought:
> movl thread_state, native_trans
> lock addl rsp-x, 0 /* dummy */
> 
> Seemed a bit silly when we could do:
> lock addl thread_state, 1
> 
> If nothing else saves an instruction. (assuming cache-lines are equally contended)
> 
> Shall I remove that change ?

No, it's fine. It was just rather baffling when i was trying to understand the
patch.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671