higan-emu / libco Goto Github PK

Using clang++-9.0.1 on Debian:

$ clang++ -x c -std=c89 libco.c
In file included from libco.c:13:
./amd64.c:14:11: error: expected parameter declarator
  alignas(4096)
          ^
./amd64.c:14:11: error: expected ')'
./amd64.c:14:10: note: to match this '('
  alignas(4096)
         ^
./amd64.c:14:3: warning: declaration specifier missing, defaulting to 'int'
  alignas(4096)
  ^
  int
./amd64.c:14:16: error: expected ';' after top level declarator
  alignas(4096)
               ^
               ;
1 warning and 3 errors generated.

g++-10.2.0 works fine.

Building in C99 mode works properly for both clang and gcc.

If we implement co_switch as a #define to a GCC __asm__, instead of the current machine code trickery, we can let GCC deal with spilling the registers - and, more importantly, not spilling registers that are currently unused.

I believe this would yield a performance boost (though most likely small, sometimes zero depending on platform and how the callers look). The drawback is an ifdef in the header (can't put it in the backend, it'd just make gcc dump every callee preserve reg to the stack, just like the current backends).

Does this sound like an interesting optimization, or would ifdefs in headers be unappealing? If yes, I'll implement it, check how bsnes performance reacts, and submit a PR later today or tomorrow.

(GCC ASM obviously won't work on MSVC, but we can keep the current backends for that. Will work on Clang, though probably not clang-cl.)

Originally from higan-emu/higan#51. I can contribute a version for amd64, sjlj, ucontext, and probably x86 too. But someone would have to handle arm, ppc, winfiber.

Additional consideration: there should probably also be a callback to destroy the userdata. Does that clutter the API too much? Should there be co_create and co_create_ext, where the latter accepts userdata+destructor?

LIBCO_NO_SSE makes libco clobber the SSE registers, contrary to the win64 ABI.

Intuitively, you'd think that's safe if you don't use any floating-point types, but no - the compiler can invent use of SIMD regs from integer operations, and expect them to stay alive across function calls. https://godbolt.org/z/hbr8aaeK4

Even more insidiously, even if there's nothing to vectorize, compilers can spill %rax to %xmm0 instead of to stack. https://godbolt.org/z/dz5f3a36o I wasn't able to convince GCC to spill xmm6 to stack then spill anything else to xmm6, and Clang seems disinclined to spill to xmm at all, but that promises nothing about newer compilers.

The only safe way to use LIBCO_NO_SSE is compiling large parts of your program with -ffixed-xmm6 -ffixed-xmm7 -ffixed-xmm8 -ffixed-xmm9 -ffixed-xmm10 -ffixed-xmm11 -ffixed-xmm12 -ffixed-xmm13 -ffixed-xmm14 -ffixed-xmm15. And for extra fun, Clang and MSVC don't support those options. (More specifically, on entry to co_switch inside a coroutine, every function on the current stack must have those flags. It's fine to call external code, as long as no callbacks can call co_switch.)

Valgrind is a great way to diagnose memory-safety issues, but it doesn't like libco very much - when libco switches to a new stack, Valgrind assumes that the entire span of memory from the old stack pointer to the new stack pointer is stack memory, and gets very confused.

Apparently Valgrind has a client request API that lets you tell valgrind "this region of memory is a new, unconnected stack" which should make valgrind much more reliable.

Here and here are examples of the API in use.

To integrate this properly with libco, we would need to:

• copy the headers into the libco repository (they're BSD licensed, specifically to make this possible)
• add the relevant calls to creating and destroying stacks

I don't think we need to care about including the header only on supported platforms (the header is pretty good about platform detection as-is) or about including it only in debug builds (apparently the overhead is ~7 instructions, and creating/destroying coroutines shouldn't be on any hot path).

The Valgrind docs for the stack annotations say "Warning: Unfortunately, this client request is unreliable and best avoided" but they don't suggest any real alternative.

Sorry for this dummy question. According to the usage document, a coentry() function must not return and should instead co_switch() to another cothread before it ends. But since it's now like an infinite loop of thread jumping, how should we "exit" such a program properly then (calling exit() works though, I am just not sure if it's also an undefined behavior). Just for example, my code is like

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include "libco.h"

void cothr_a_job();
void main_cothr_job();

cothread_t main_cothr;

int main()
{
    if ((main_cothr = co_create(1024 * sizeof(void *), main_cothr_job))) {
        printf("main cothread started\n");
    } else {
        fprintf(stderr, "failed to create main cothread\n");
    }

    co_switch(main_cothr);

    return 0; // will never reach here
}

void main_cothr_job()
{
    printf("entering main cothread!\n");
    
    cothread_t a;
    if ((a = co_create(1024 * sizeof(void *), cothr_a_job))) {
        printf("thread A started\n");
    } else {
        fprintf(stderr, "failed to create thread A\n");
    }

    co_switch(a);
    printf("back to main cothread\n");
    co_delete(a);
    printf("thread A deleted\n");
    printf("leaving the program\n");
    exit(0); // is it the correct way to end such a program?
}

void cothr_a_job()
{
    printf("Hi from cothread A\n");
    co_switch(main_cothr);
    sleep(200);
}

There's a couple different takes on the ISC license with slight modifications to the terminology. To avoid potential confusion, it would be nice to have the ISC license included in the repository.

I presume you intend to use this version:

Copyright

Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above copyright notice and this permission notice appear in all copies.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

But there's another, older variant with some slight differences the and/or, and just and.

Wikipedia reference: https://en.wikipedia.org/wiki/ISC_license

Hi there, I'm trying to compile the example test_timing.cpp under Windows. MSys2+MinGW-gcc worked.
With VS2019, however, I get

C2341 segment must be defined using #pragma data_seg, code_seg or section prior to use

in amd64.c here

If I change it to

  #ifdef _MSC_VER
    #pragma data_seg(".text")
  #endif

I get libco.obj : warning LNK4078: multiple '.text' sections found with different attributes (60500020).
Please advise.
Thank you!

Hi there

I would be interested to compile bsnes to a RISC V machine.

Do you have any plans to support that platform?

It's an open source architecture, backed by nearly everyone, except ARM :P

Is this a lot of work?

The current version of the libco repository is just a snapshot of the libco code as it was in the higan repo as of even date. It seems a shame to lose all the development history unnecessarily, and it's not too difficult to git subtree split it out. I propose we do that, and update this repo before too many people start git submoduling this commit ID into their code.

However, some questions remain:

• how much history to include?
  • we could just take all the history from the higan repo, which is reliable but incomplete
  • we could include history reconstructed from old source archives, which might be misleading if more source archives are discovered later (we don't want to rewrite history to include them)
• how do we update master?
  • if we force-push the new commit history, it will be clean, but could disrupt possible downstreams
  • if we merge the commit history, it will be weird glitch in the history, but it at least it should be simple for downstream users to follow the history.

If the input pointer is not 16-aligned, this will give the wrong answer, and execute code on a misaligned stack.

I believe it can be fixed by changing the line to unsigned int offset = (size & ~15) - 32 - (15&(uintptr_t)memory);, but I didn't test this.

Fair chance the same bug exists on other platforms too, I didn't check.

It seems that it is used only to get a unique address when calling co_active from a non-coroutine function. Why is it an array then, and not just a thread-local global variable?