On Mon, Dec 07, 2015 at 01:19:07PM +0300, Nick Zavaritsky <mej...@gmail.com> 
wrote:
> > 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.

Ah, but making it fail is not playing it safe - it's of no consequence at the
moment, but I tightened the checks, just to be safe :)

The main point here is that at one point, I will want to enable CORO_ASM
automatically on supported arm builds, and then I need to know how to
properly detect those.

> > 2. The streamlined CORO_STARTUP code is something I'd like to avoid unless
>
> 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?

A bit of both, but mostly maintainability is my concern. By using the
portable (startup) code for all cases, the amount of code that needs to be
maintained is reduced, and, most importantly, the generic startup is
exercised a lot more than an architecture-specific one.

> > 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.

Good, so your compiler also accepts regparm, as it should.

> 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. 

Thanks for the pointer. Indeed, if an fpu were present, we could save/restore
just in case, but I suspect the code would still get to run on softfp
machines, so "conservative" would fail.

Can you try this patch and see if it works for your config(s)? Thanks!

http://data.plan9.de/asm-arm.diff

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      schm...@schmorp.de
      -=====/_/_//_/\_,_/ /_/\_\

_______________________________________________
libev mailing list
libev@lists.schmorp.de
http://lists.schmorp.de/mailman/listinfo/libev

Reply via email to