Skip to content

Commit 424ad16

Browse files
authored
[flang] Simplify LIBTYPE logic (#98072)
When the `add_flang_library` was first added, it was apparently copied over from `add_clang_library`, including its logic to determine the library type. It includes a workaround: If `BUILD_SHARED_LIBS` is enabled, it should build all libraries as shared, including those that are explicitly marked as `STATIC`[^1], because `add_clang_library` always passes at least one of `STATIC`/`SHARED` to `llvm_add_library`, and `llvm_add_library` could not distinguish the two cases. Then, the two implementations diverged. For its runtime libraries, Flang requires some libraries to always be static libraries, so if a library is explicitly marked as `STATIC`, `BUILD_SHARED_LIBS` is ignored[^2].   I noticed the two implementations of the same functionality, modified only the `add_clang_library`, and copied over the result to `add_flang_library`[^3], without noticing that they are slightly different. As a result, Flang runtime libraries would be built as shared libraries with `-DBUILD_SHARED_LIBS=ON`, which may break some build configurations[^4]. This PR fixes the problem and at the same time simplifies the library type algorithm by just passing SHARED/STATIC verbatim to `llvm_add_library`. This is effectively what [^2] should have done instead adding more code to undo the workaround of [^1]. Ideally, one would use ``` llvm_add_library(${name} ${ARG_STATIC} ${ARG_SHARED} [...]) ``` but `ARG_STATIC`/`ARG_SHARED` as set by `cmake_parse_arguments` contain `TRUE`/`FALSE` instead of the keywords themselves. I could imagine a utility function akin to `pythonize_bool` that does this. This simplification adds two more changes: 1. Object libraries are not explicitly requested anymore. `llvm_add_library` itself should determine whether an object library is necessary. As the comment notes, using an object library is not without problems and seem of no use here since it works fine without object library when in `XCODE`/`MSVC_IDE` mode. 2. The property `CLANG_STATIC_LIBS` was removed. It was `FLANG_STATIC_LIBS` before to copy&paste error of #93519 [^3] which not used anywhere. In clang, `CLANG_STATIC_LIBS` is used for `clang-shlib` to include all component libraries in a single large library. There is no equivalent `flang-shlib`. [^1]: dbc2a12 [^2]: 3d2e05d [^3]: #93519 [^4]: #93519 (comment)
1 parent 77fd30f commit 424ad16

File tree

1 file changed

+6
-16
lines changed

1 file changed

+6
-16
lines changed

flang/cmake/modules/AddFlang.cmake

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,23 +55,13 @@ function(add_flang_library name)
5555
set(LIBTYPE SHARED STATIC)
5656
elseif(ARG_SHARED)
5757
set(LIBTYPE SHARED)
58+
elseif(ARG_STATIC)
59+
# If BUILD_SHARED_LIBS and ARG_STATIC are both set, llvm_add_library prioritizes STATIC.
60+
# This is required behavior for libFortranFloat128Math.
61+
set(LIBTYPE STATIC)
5862
else()
59-
# llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
60-
# so we need to handle it here.
61-
if(BUILD_SHARED_LIBS)
62-
set(LIBTYPE SHARED)
63-
else()
64-
set(LIBTYPE STATIC)
65-
endif()
66-
if(NOT XCODE AND NOT MSVC_IDE)
67-
# The Xcode generator doesn't handle object libraries correctly.
68-
# The Visual Studio CMake generator does handle object libraries
69-
# correctly, but it is preferable to list the libraries with their
70-
# source files (instead of the object files and the source files in
71-
# a separate target in the "Object Libraries" folder)
72-
list(APPEND LIBTYPE OBJECT)
73-
endif()
74-
set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
63+
# Let llvm_add_library decide, taking BUILD_SHARED_LIBS into account.
64+
set(LIBTYPE)
7565
endif()
7666
llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
7767

0 commit comments

Comments
 (0)