[PATCH] D70972: [AIX] Make sure we use export lists for plugins

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/CMakeLists.txt:859
+  string(REPLACE "-Wl,-brtl" "" CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS}")
+  string(REPLACE "-Wl,-brtl" "" CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS}")
 endif()
----------------
The removal of `-brtl` from `CMAKE_EXE_LINKER_FLAGS` leads to test failures (with SIGILL) for various loadable-module tests such as `llvm/test/Feature/load_module.ll`.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:913
 # This option makes utils/extract_symbols.py be used to determine the list of
-# symbols to export from LLVM tools. This is necessary when using MSVC if you
-# want to allow plugins, though note that the plugin has to explicitly link
-# against (exactly one) tool so we can't unilaterally turn on
-# LLVM_ENABLE_PLUGINS when it's enabled.
-option(LLVM_EXPORT_SYMBOLS_FOR_PLUGINS "Export symbols from LLVM tools so that plugins can import them" OFF)
+# symbols to export from LLVM tools. This is necessary when on AIX or using MSVC
+# if you want to allow plugins. On AIX we don't show the option, and enable this
----------------
It helps me read this easier if "when" is inserted before "using MSVC".


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:914
+# symbols to export from LLVM tools. This is necessary when on AIX or using MSVC
+# if you want to allow plugins. On AIX we don't show the option, and enable this
+# by default except when the LLVM libraries are set up for dynamic linking, due
----------------
If I understand correctly, the "initial configuration" aspect has now been corrected.

Suggestion:
On AIX we don't show **this** option, and **we** enable it by default except when the LLVM libraries are set up for dynamic linking **(**due to incompatibility**)**.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:916
+# by default except when the LLVM libraries are set up for dynamic linking, due
+# to incompatibility in the initial configuration. With MSVC note though that 
+# the plugin has to explicitly link against (exactly one) tool so we can't 
----------------
With MSVC**,** note that [ ... ].


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:919
+# unilaterally turn on LLVM_ENABLE_PLUGINS when it's enabled.
+set(LLVM_EXPORT_SYMBOLS_FOR_PLUGINS_default OFF)
+if (NOT (BUILD_SHARED_LIBS OR LLVM_LINK_LLVM_DYLIB))
----------------
This seems misnamed, as this should just be the default for AIX.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70972/new/

https://reviews.llvm.org/D70972