jpbruyere / vkhelpers

Vulkan c helper library
MIT License
32 stars 10 forks source link

Implmented switch between Tinycthread and Threads.h #9

Open LinuxLegend opened 2 years ago

LinuxLegend commented 2 years ago

I've added small change to ensure build process would use Tinycthread for Windows and use Threads.h from C11 standard for all other platforms.

jpbruyere commented 2 years ago

it would be better to test 'STDC_NO_THREADS' macro.

LinuxLegend commented 2 years ago

Alright, switched over to STDC_NO_THREADS

jpbruyere commented 2 years ago

I came across this: https://stackoverflow.com/questions/61887795/gcc-is-non-conforming , also build systems must also conform to this check.

LinuxLegend commented 2 years ago

It seems that GCC would actually have Threads.h if you set the flag for C11 standard which is what Cmake and Meson does. Also added further checks for STDC version.

LinuxLegend commented 2 years ago

Also I am digging around for Meson/CMake on threads support.

LinuxLegend commented 2 years ago

Maybe we should just opt to just give Meson/Cmake a try to buildng a tiny software and see if Threads.h is available if not, it'll switch back to Tinycthread if it isn't available?

jpbruyere commented 2 years ago

in cmake, there's the possibility to check symbols, another option is to include a threading header for both cases that will do the test on compilation time. Also note that vkvg is not compiling with the meson build because you don't include tinycthread, and so you miss some macro definition done by it.

LinuxLegend commented 2 years ago

Yeah, we need to update in this library here first and I can then split up the meson build over at vkvg going forward and then hopefully put the tinycthread issue at rest between vkh and vkvg.

LinuxLegend commented 2 years ago

@jpbruyere I have tried to introduce a tiny snippet for checking threads and it turns out that Meson fail to support that particular feature which would've allowed us to check whether or not threads was supported.

image

# Do a Threads.h check
# Original lines were:
# hasthreads_test = executable('hasthreads', 'verify/hasthreads.c', dependencies: [ threads_dep ], build_by_default: true)
# hasthreads_result = run_command(hasthreads_test, check: false)
hasthreads_result = run_command(executable('hasthreads', 'verify/hasthreads.c', dependencies: [ threads_dep ], build_by_default: true), check: false)

has_c11_threads_support = false
has_c11_threads_support_defines = []
if (hasthreads_result.returncode() == 0)
    has_c11_threads_support = true
    has_c11_threads_support_defines += '-DHAS_C11_THREADS=1'
endif

hasthreads.c

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

static volatile int retVal = 1;

int validate(void* arg)
{
    retVal = 0;
    return 0;
}

int main()
{
    thrd_t thread;
    if (thrd_create(&thread, validate, NULL) != thrd_success) return 1;
    int res;
    if (thrd_join(thread, &res) != thrd_success) return 1;
    return retVal;
}
LinuxLegend commented 2 years ago

I could write 2 scripts for Windows and Linux to run a test, would that work?

jpbruyere commented 2 years ago

I like simplicity, If I would do this task, I guess I'll simply use the STDC_NO_THREADS , maybe directly in tinycthread header and code.

LinuxLegend commented 2 years ago

I thought about doing that as well, turns out that in some of the channels I have spoken to about C compiler, some of the people mentioned that some compilers would provide threads.h header, but not the actual API required to do this. So that why I was thinking about writing the scripts to actually try and compile to see if it works at all, if not, then we'll know to use Tinycthread going forward.

jpbruyere commented 2 years ago

Take time to find the best solution, and try to keep it simple. The test program looks like to me a heavy solution for a simple problem.

LinuxLegend commented 2 years ago

Would this do?