coro: Asm coro_transfer for arm7

Nick Zavaritsky mejedi at gmail.com
Mon Dec 7 11:19:07 CET 2015


Hi!

> 1. in the header, you test for __arm__, but in the implementation,
> you only support __ARM_ARCH=7, this will obviously fail. Is this an
> oversight? I would assume testing for __ARM_ARCH everywhere is the right
> fix?

I am lacking the capacity to test it on every possible arm; so just playing it safe here. There is nothing arm7-specific in the code.

> 2. The streamlined CORO_STARTUP code is something I'd like to avoid unless
> it's superuseful (I'd like to avoid the complications because I don't
> think they are worth the speed gained, unless it would be substantial) -
> do you have any benchmark or some other data that caused you to optimise
> this case, or did you just do it while you were at it?

I don’t think it’s super useful; did that primarily for aesthetic reasons. Extra coroutine switching and passing parameters via globals feels so redundant when we have plenty of registers saved and restored. Just wondering: do you consider an added assembly chunk a complication or is it series of ifdefs mangling the surrounding code?

> 3. I assume your change to not use regparm on arm is merely to avoid a
> compiler warning?

Looks like arm abi specifically requires passing parameters via registers; i.e. there is only one calling convention and regparam does nothing except producing a warning.

> 4. It seems the __ARM_ARCH==7 test is not quite good enough, I get:
> 
> Just guessing, but something along the lines of this might do for
> coro_transfer? (I haven't been able to test it):
> 
>         #if __VFP_FP__
>           "\tvpush {d8-d15}\n"
>           #define NUM_SAVED (9 + 8 * 2)
>         #else
>           #define NUM_SAVED 9
>         #endif
>         "\tpush {r4-r11,lr}\n"
>         "\tstr sp, [r0]\n"
>         "\tldr sp, [r1]\n"
>         "\tpop {r4-r11,lr}\n"
>         #if __VFP_FP__
>           "\tvpop {d8-d15}\n"
>         #endif
>         "\tmov r15, lr\n”

https://code.google.com/p/v8/issues/detail?id=2140

__VFP_FP__ is merely a test if FPU is present. Is is recommended to check __ARM_PCS_VFP instead; but there are gotchas. 

Regards.


More information about the libev mailing list