[flang-commits] [flang] [flang] Simplify LIBTYPE logic (PR #98072)

[Michael Kruse via flang-commits]

https://github.com/Meinersbur created https://github.com/llvm/llvm-project/pull/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 case.

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 the build[^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.

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 find with `XCODE`/`MSVC_IDE`.

2. The property `CLANG_STATIC_LIBS` is removed. It was `FLANG_STATIC_LIBS` before to copy&paste error of #93519 [^3] but not used anywhere[^5].

[^1]: dbc2a12c7311ff4cc2cd7887d128b506bd35b579

[^2]: 3d2e05d542e646891745c5278a09950d3c4fb4a5

[^3]: #93519

[^4]: https://github.com/llvm/llvm-project/pull/93519#issuecomment-2192359002

[^5]: In clang, `CLANG_STATIC_LIBS` is used for `clang-shlib` to include all component libraries in a single large library. There is no `flang-shlib`.

>From 426fd0339514b2c25f141ca85b95e6e83a56af65 Mon Sep 17 00:00:00 2001
From: Michael Kruse <llvm-project at meinersbur.de>
Date: Mon, 8 Jul 2024 19:57:26 +0200
Subject: [PATCH] [flang] Simplify LIBTYPE logic

---
 flang/cmake/modules/AddFlang.cmake | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/flang/cmake/modules/AddFlang.cmake b/flang/cmake/modules/AddFlang.cmake
index aeb4d862cf780..9ed1a3050b7e8 100644
--- a/flang/cmake/modules/AddFlang.cmake
+++ b/flang/cmake/modules/AddFlang.cmake
@@ -55,23 +55,13 @@ function(add_flang_library name)
     set(LIBTYPE SHARED STATIC)
   elseif(ARG_SHARED)
     set(LIBTYPE SHARED)
+  elseif(ARG_STATIC)
+    # If BUILD_SHARED_LIBS and ARG_STATIC are both set, llvm_add_library prioritizes STATIC.
+    # This is required behavior for libFortranFloat128Math.
+    set(LIBTYPE STATIC)
   else()
-    # llvm_add_library ignores BUILD_SHARED_LIBS if STATIC is explicitly set,
-    # so we need to handle it here.
-    if(BUILD_SHARED_LIBS)
-      set(LIBTYPE SHARED)
-    else()
-      set(LIBTYPE STATIC)
-    endif()
-    if(NOT XCODE AND NOT MSVC_IDE)
-      # The Xcode generator doesn't handle object libraries correctly.
-      # The Visual Studio CMake generator does handle object libraries
-      # correctly, but it is preferable to list the libraries with their
-      # source files (instead of the object files and the source files in
-      # a separate target in the "Object Libraries" folder)
-      list(APPEND LIBTYPE OBJECT)
-    endif()
-    set_property(GLOBAL APPEND PROPERTY CLANG_STATIC_LIBS ${name})
+    # Let llvm_add_library decide, taking BUILD_SHARED_LIBS into account.
+    set(LIBTYPE)
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})