Thread overview
[Bug 266] Can threadasm.S be replaced with generics?
Jul 19, 2017
Johannes Pfau
Jul 19, 2017
Iain Buclaw
Jul 19, 2017
Iain Buclaw
Jul 19, 2017
Johannes Pfau
July 19, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=266

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |johannespfau@gmail.com

--- Comment #1 from Johannes Pfau <johannespfau@gmail.com> ---
Interesting. Is there any documentation for these functions?
I'm wondering whether these functions are always guaranteed to save/restore all
registers? And why is __builtin_unwind_init required? Is this used to mark
registers as 'live' as written in some exception documentation?

We probably have to be careful when calling the builtin directly: https://github.com/gcc-mirror/gcc/blob/master/gcc/ipa-split.c#L928 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31128

Also generated ARM code looks basically good, but there's one thing I don't
understand:
https://explore.dgnu.org/g/U2QTsK

        push    {r4, r5, r6, r7, r8, r9, r10, fp}
        add     fp, sp, #28
        str     sp, [r0]
        mov     sp, r1
        sub     sp, fp, #28
        pop     {r4, r5, r6, r7, r8, r9, r10, fp}
        bx      lr

What is the add/sub fp code supposed to do? Doesn't this actually restore the old stack pointer (have to look up the ARM assembler operand order, but isn't add.. => fp = sp+28 and sub... => sp = fp-28 which restores the old sp?).

-- 
You are receiving this mail because:
You are watching all bug changes.
July 19, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=266

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> ---
(In reply to Johannes Pfau from comment #1)
> What is the add/sub fp code supposed to do? Doesn't this actually restore the old stack pointer (have to look up the ARM assembler operand order, but isn't add.. => fp = sp+28 and sub... => sp = fp-28 which restores the old sp?).

Hmm, yeah you're right.  It would all be fine if it weren't for the restoring of the FP.  Having a look again, there is the same in the x86 version too.


        mov     QWORD PTR [rdi], rsp
        mov     rsp, rsi
        lea     rsp, -40[rbp]  <-- Here.


So at best it makes a good indicator of what you should go threadasm,  but *watch out* for FP restoring.

So this is would be for documentation purposes then.

https://explore.dgnu.org/g/z3dB2R

-- 
You are receiving this mail because:
You are watching all bug changes.
July 19, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=266

--- Comment #3 from Iain Buclaw <ibuclaw@gdcproject.org> ---
(In reply to Iain Buclaw from comment #2)
> (In reply to Johannes Pfau from comment #1)
> > What is the add/sub fp code supposed to do? Doesn't this actually restore the old stack pointer (have to look up the ARM assembler operand order, but isn't add.. => fp = sp+28 and sub... => sp = fp-28 which restores the old sp?).
> 
> Hmm, yeah you're right.  It would all be fine if it weren't for the restoring of the FP.  Having a look again, there is the same in the x86 version too.
> 
> 
>         mov     QWORD PTR [rdi], rsp
>         mov     rsp, rsi
>         lea     rsp, -40[rbp]  <-- Here.
> 
> 

And this explains why the call to stack_restore() was optimized away entirely without a compiler level memory barrier.  I was rather puzzled by that last night.

-- 
You are receiving this mail because:
You are watching all bug changes.
July 19, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=266

--- Comment #4 from Johannes Pfau <johannespfau@gmail.com> ---
Actually there's one more thing to watch out for:
The ARM code does not save & restore the link register. The context switch is
supposed to also switch the 'call stack' so this is invalid as well, as the
function will return to the original caller and not the caller belonging to the
newp* argument.

Nevertheless, an intrinsic approach would be much cleaner and the compiler already knows all required backend / ABI information anyway. So maybe once we're merged into GCC we can introduce a new __builtin_switch_context for this?

Actually, shouldn't Go have something similar for their goroutine stuff?

-- 
You are receiving this mail because:
You are watching all bug changes.