-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[flang-rt] Use --as-needed for linking flang-rt libraries. #130856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change makes sure that there is no unnecessary library dependencies like libc++/libstdc++.
I don't it does what you think it does. The static library is just a collection of object files. Only when linking the executable you actually know what "is needed". Until then, you want all the object files in the library. We actually have a test that checks that no libc++ dependencies are required: https://github.com/llvm/llvm-project/blob/main/flang-rt/test/Runtime/no-cpp-dep.c |
It works for the shared libraries build, I checked. I know that we have the test, but the library dependencies are still established by the linker, and they need to be satisfied. Which makes it awkward that we need to link c++ library for Fortran programs. |
I think thats what
If this is for the shared library, please only apply it to the shared libary. This can be done by moving it into the
|
At least, could you add to the comment that this is for the shared library only? For the static library, the linker is not even used (CMAKE_AR is being used) and LINKER_LANGUAGE/LINK_FLAGS being ignored. |
Unfortunately, it is not enough. See the comment about
Thanks! I will move the code around. Good point about the position of the option: it works now, but I will check if I can do it more robust. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This change makes sure that there is no unnecessary library
dependencies like libc++/libstdc++.