search

unikraft / lib-musl

musl: A C standard library
Other
9 stars 29 forks source link

Revert "Makefile.uk: Add flag to avoid gcc specifc symbol" #46

Closed andreittr closed 1 year ago

andreittr commented 1 year ago

This reverts commit 4912487a42ec9ab1f7ab7bcca698c50a849eafa8.

__muldc3 is implemented by lib-compiler-rt for clang, and emitting calls to it is normal. While -ffast-math prevents these calls, it also enables unsafe optimizations that might result in wrong results from math functions. Best to just pull in lib-compiler-rt when needed.

edit: also, with a newer clang (v16.0.1 on my end) musl fails to build with the -ffast-math flag:

.../apps/app-helloworld-cpp/build/libmusl/origin/musl-1.2.3//src/math/fmal.c:167:15: error: '#pragma STDC FENV_ACCESS ON' is illegal when precise is disabled
        #pragma STDC FENV_ACCESS ON
                     ^
razvand commented 1 year ago

@mariasfiraiala, we fixed the clang-building issue here: https://github.com/unikraft/unikraft/pull/877

mariasfiraiala commented 1 year ago

@mariasfiraiala, we fixed the clang-building issue here: unikraft/unikraft#877

Nice thing, thanks @razvand @StefanJum! Now, regarding the __muldc3 issue, I think @StefanJum's suggestion is the way to go, as:

  1. It's easy, and doesn't imply pulling lib-compiler-rt for every app.

  2. Even though -ffast-math might produce build errors with (very) new versions of clang, one can still pull lib-compiler-rt as an emergency and build their app nevertheless.

@andreittr what do you think?

andreittr commented 1 year ago

I wouldn't pull compiler-rt every time we compile something with musl. It looks like a lot of unneeded extra stuff. If the issue only appears when using compiler-rt, can't we do something like this?

From what I can tell, compiling musl with clang emits __muldc3 iff complex numbers are enabled (CONFIG_LIBMUSL_COMPLEX) so pulling in lib-compiler-rt is not required for all apps that use musl, only strictly for those that need complex number support.

Even though -ffast-math might produce build errors with (very) new versions of clang, one can still pull lib-compiler-rt as an emergency and build their app nevertheless.

The build errors with clang 16 are at compile-time and refer to (1) musl (via pragmas) requesting certain guarantees on floating-point maths and (2) clang being unable to provide those guarantees due to -ffast-math, so pulling in lib-compiler-rt will not help in that case (missing __muldc3 errors are at link-time, but that's much later).

edit: I agree that pulling in lib-compiler-rt (and, therefore, an entire C++ runtime) only for __muldc3 is ridiculous, and a better solution is needed. My thoughts are to split up lib-compiler-rt into the essential builtins (like implementing float ops) and extra stuff (like unwinding & what else is in gcc_personality_v0.c) but that's for another time.

razvand commented 1 year ago

edit: I agree that pulling in lib-compiler-rt (and, therefore, an entire C++ runtime) only for __muldc3 is ridiculous, and a better solution is needed. My thoughts are to split up lib-compiler-rt into the essential builtins (like implementing float ops) and extra stuff (like unwinding & what else is in gcc_personality_v0.c) but that's for another time.

@andreittr, please create an issue as part of the lib-compiler-rt repository with this.