RFR 8233500: Shenandoah: Shenandoah load barrier should save registers before calling keep alive barrier on x86

[Roman Kennke]

Am 5. November 2019 13:50:11 MEZ schrieb Zhengyu Gu <zgu at redhat.com>:
>
>
>On 11/4/19 1:39 PM, Aleksey Shipilev wrote:
>> On 11/4/19 7:34 PM, Zhengyu Gu wrote:
>>>> SATB handling is similar to G1 is doing, where's the similar code
>in G1? The patch adds save/restore
>>>> at in SBSA::load_at, but there is a similar block in
>SBSA::store_at, why it is not needed there?
>>>
>>> Because we do self-fixing in LRB and have to reshuffle registers.
>> 
>> Okay. So AArch64 does enter()/leave(), why x86 needs the entire
>IU_state pushed/popped?
>> 
>> My concern is that pushing/popping the entire state explodes code
>size (we don't care about
>> performance much, but we do care about hitting the stub boundaries),
>and probably hides some bugs
>> with register shuffles.
>> 
>
>push_IU_state()/pop_IU_state(), each adds 3 instructions on x86_64, 2
>on 
>x86_32.
>
>
>void MacroAssembler::push_IU_state() {
>   // Push flags first because pusha kills them
>   pushf();
>   // Make sure rsp stays 16-byte aligned
>   LP64_ONLY(subq(rsp, 8));
>   pusha();
>}
>
>
>void MacroAssembler::pop_IU_state() {
>   popa();
>   LP64_ONLY(addq(rsp, 8));
>   popf();
>}
>
>-Zhengyu

Careful there! IIRC pusha() is a macro instruction on x86_64 that expands to 16+ instructions. 

On the other side, we did examine in the past to reduce instructions there by analysing what the interpreter actually uses and what is required by calling conventions, and yes there are opportunities to reduce number of instructions, but the risk of bugs due to the barrier being called from unusual places, e.g. via C2/cas-obj or outside interpreter (we did have such bugs and they are not fun!) did not make it seem worth. Saving/restoring the entire state seems conservatively save approach.

Roman


-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.