[AIX][cmake] Adjust management of `-G` for linking

[hubert.reinterpretcast]

Summary

The change in 0ba98433971f changed the behaviour of the build when using an XL build compiler because -G is not a pure linker option: it also implies -shared. This was accounted for in the base CMake configuration, so an analysis of the change from 0ba98433971f in relation to a build using Clang (where -shared is introduced by CMake) would not identify the issue. This patch resolves this particular issue by adding -shared alongside -Wl,-G.

At the same time, the investigation reveals that several aspects of the various build configurations are not operating in the manner originally intended.

The other issue related to the -G linker option in the build is that the removal of it (to avoid unnecessary use of run-time linking) is not effective for the build using the Clang compiler. This patch addresses this by adjusting the regular expressions used to remove the broadly-applied -G.

Finally, the issue of specifying the export list with -Wl, instead of a compiler option is flagged with a FIXME comment.

[amyk]

Thanks for investigating this Hubert. Overall makes sense and LGTM, as long as there are no other concerns from anyone else.

[daltenty]

LGTM, thanks!

llvm/CMakeLists.txt

939

An aside: we should consider if this is worth mentioning to upstream CMake.

llvm/cmake/modules/AddLLVM.cmake

94

Just an aside note, very recent version of CMake on AIX now write their own export lists. When this gets fixed, it would also be good to check the interactions with that.

[hubert.reinterpretcast]

llvm/CMakeLists.txt

939

@daltenty, this is building on your prior work to reduce the run-time linking exposure. It seems upstream CMake does understand the need on AIX for -shared -Wl,-G for non-XL and -G for XL when the intent is to use run-time linking. That is, out-of-the-box, I think CMake works if we were happy with the run-time linking. It seems the ability for a user to communicate the run-time linking intent in terms of CMake may be where there is a gap.

[daltenty]

llvm/CMakeLists.txt

939

The runtime linking aspect was fixed in CMake 3.16: https://gitlab.kitware.com/cmake/cmake/-/issues/19163 . After that version -G / runtime linking is dropped from the default options.

I'm more referring to the fact that CMAKE_MODULE_LINKER_FLAGS seems like it should probably contain -shared out-of-the-box

[hubert.reinterpretcast]

llvm/CMakeLists.txt

939

I'm not sure CMAKE_MODULE_LINKER_FLAGS is or is not the right level in CMake. For the LLVM build, I think putting it in CMAKE_MODULE_LINKER_FLAGS is consistent with a more manually-tuned build (but actually only when using an XL compiler at this time). Since the builds on AIX are still in flux, I left the -shared from https://gitlab.kitware.com/cmake/cmake/-/blob/5988a4deea67ed122b30c2c0437f70e074273789/Modules/Compiler/GNU.cmake#L36 alone for the builds using a Clang build compiler.

For general CMake, I think the lack of -qmkshrobj in a similar position in the base configuration when using an XL build compiler is a good starting point for the discussion.

llvm/cmake/modules/AddLLVM.cmake

94

We need to check both that the LLVM mechanism can drive/override the CMake management of export lists and that the CMake mechanism is respected by the compiler's linker invocation. For GCC on AIX, I think CMake might need to avoid using -shared.

llvm/CMakeLists.txt

if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
           add_definitions("-D_LARGE_FILE_API")
 
   # CMake versions less than 3.16 set default linker flags to include -brtl, as
   # well as setting -G when building libraries, so clear them out. Note we only
   # try to clear the form that CMake will set as part of its initial
   # configuration, it is still possible the user may force it as part of a
   # compound option.
   if(CMAKE_VERSION VERSION_LESS 3.16)
-    string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" "" CMAKE_EXE_LINKER_FLAGS  "${CMAKE_EXE_LINKER_FLAGS}")
-    string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" "" CMAKE_SHARED_LINKER_FLAGS  "${CMAKE_SHARED_LINKER_FLAGS}")
-    string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" "" CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS}")
-    string(REGEX REPLACE "(^|[ \t]+)(-Wl,)?-G([ \t]+|$)" "" CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS
+    string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" " " CMAKE_EXE_LINKER_FLAGS  "${CMAKE_EXE_LINKER_FLAGS}")
+    string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" " " CMAKE_SHARED_LINKER_FLAGS  "${CMAKE_SHARED_LINKER_FLAGS}")
+    string(REGEX REPLACE "(^|[ \t]+)-Wl,-brtl([ \t]+|$)" " " CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS}")
+    string(REGEX REPLACE "(^|[ \t]+)(-Wl,)?-G([ \t]+|$)" " " CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS
       "${CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS}")
-    string(REGEX REPLACE "(^|[ \t]+)(-Wl,)?-G([ \t]+|$)" "" CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS
+    string(REGEX REPLACE "(^|[ \t]+)(-Wl,)?-G([ \t]+|$)" " " CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS
       "${CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS}")
-    string(REGEX REPLACE "(^|[ \t]+)(-Wl,)?-G([ \t]+|$)" "" CMAKE_SHARED_LIBRARY_CREATE_ASM_FLAGS
+    string(REGEX REPLACE "(^|[ \t]+)(-Wl,)?-G([ \t]+|$)" " " CMAKE_SHARED_LIBRARY_CREATE_ASM_FLAGS
       "${CMAKE_SHARED_LIBRARY_CREATE_ASM_FLAGS}")
+    string(REGEX REPLACE "(^|[ \t]+)-Wl,-G," " -Wl," CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS
+      "${CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS}")
+    string(REGEX REPLACE "(^|[ \t]+)-Wl,-G," " -Wl," CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS
+      "${CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS}")
+    string(REGEX REPLACE "(^|[ \t]+)-Wl,-G," " -Wl," CMAKE_SHARED_LIBRARY_CREATE_ASM_FLAGS
+      "${CMAKE_SHARED_LIBRARY_CREATE_ASM_FLAGS}")
   endif()
 
-  # Modules should be built with -G, so we can use runtime linking with
-  # plugins.
-  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -Wl,-G")
+  # Modules should be built with -shared -Wl,-G, so we can use runtime linking
+  # with plugins.
+  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -shared -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")
 endif()
 
 # Build with _XOPEN_SOURCE on z/OS.
 if (CMAKE_SYSTEM_NAME MATCHES "OS390")
   add_definitions("-D_XOPEN_SOURCE=600")

llvm/cmake/modules/AddLLVM.cmake

if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
     add_custom_command(OUTPUT ${native_export_file}
       COMMAND sed -e "s/^/_/" < ${export_file} > ${native_export_file}
       DEPENDS ${export_file}
       VERBATIM
       COMMENT "Creating export file for ${target_name}")
     set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                  LINK_FLAGS " -Wl,-exported_symbols_list,\"${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}\"")
   elseif(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+    # FIXME: `-Wl,-bE:` bypasses whatever handling there is in the build
+    # compiler driver to defer to the specified export list.
     set(native_export_file "${export_file}")
     set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                  LINK_FLAGS " -Wl,-bE:${export_file}")
   elseif(LLVM_HAVE_LINK_VERSION_SCRIPT)
     # Gold and BFD ld require a version script rather than a plain list.
     set(native_export_file "${target_name}.exports")
     # FIXME: Don't write the "local:" line on OpenBSD.
     # in the export file, also add a linker script to version LLVM symbols (form: LLVM_N.M)