riscv-software-src / riscv-isa-sim

Spike, a RISC-V ISA Simulator
Other
2.38k stars 839 forks source link

Make softfloat's rounding mode thread-local #1689

Closed aswaterman closed 3 months ago

aswaterman commented 3 months ago

This has no effect on Spike itself, but it might matter for anyone who's using Spike as a library in a multithreaded program.

aswaterman commented 3 months ago

Seems like Mac OS doesn't like linking C++ thread-local variable references against C thread-local variable definitions. I'll poke at it more later, but I won't spend much time on it.

aswaterman commented 3 months ago

OK, so here's the story:

For GCC on Linux, _Thread_local is rejected when targeting C++, so we want to either use thread_local everywhere, or conditionalize which keyword we use based on language.

For Clang on Mac OS, thread_local is rejected when targeting C, so we want to either use _Thread_local everywhere, or conditionalize which keyword we use based on language.

So we go down the road of conditionalizing which keyword we use based on language, but then we find out that Clang on Mac OS generates incompatible code when using thread_local in C++ code to access _Thread_local variables defined in C code.

One incantation that does the right thing everywhere is to check whether threads.h exists, in which case we can use thread_local even in C, and otherwise use _Thread_local. I don't think there's any guarantee that this works on all platforms, but it works for Clang and GCC on Linux and Clang on Mac OS, so I'm going to run with it. We'll see if anyone complains.

AnaBSF commented 3 months ago

This became a problem for me, as I get the following error:

error: ‘_Thread_local’ does not name a type; did you mean ‘thread_local’? 57 | # define THREAD_LOCAL _Thread_local

I am using GCC on Ubuntu.

aswaterman commented 3 months ago

Which version of GCC are you using? I tried GCC 8.5, 10.1, 12.3, and 13.2, all on Ubuntu, and they all work.

aswaterman commented 3 months ago

You can disregard that question, assuming #1704 does indeed fix your problem.