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

[via flang-commits]

Author: Michael Kruse
Date: 2024-07-12T16:11:55+02:00
New Revision: 424ad163c715e478d37cfd1efaae148cdd6aa815

URL: https://github.com/llvm/llvm-project/commit/424ad163c715e478d37cfd1efaae148cdd6aa815
DIFF: https://github.com/llvm/llvm-project/commit/424ad163c715e478d37cfd1efaae148cdd6aa815.diff

LOG: [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]: dbc2a12c7311ff4cc2cd7887d128b506bd35b579

[^2]: 3d2e05d542e646891745c5278a09950d3c4fb4a5

[^3]: #93519

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

Added: 
    

Modified: 
    flang/cmake/modules/AddFlang.cmake

Removed: 
    


################################################################################
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})