[CMake][AIX] Fixing AIX rpath

[qiongsiwu1]

Summary

Recent commit https://github.com/llvm/llvm-project/commit/8f833f88ab78265a8e0ebb0d1522771d67c708a9 modified the installation rpath and did not set BUILD_WITH_INSTALL_RPATH correctly on AIX, which led to installation failures on AIX. This patch sets BUILD_WITH_INSTALL_RPATH on AIX to fix the installation failures.

[daltenty]

LGTM, thanks

Context: the tool chain doesn't support modifying rpaths/libpaths for XCOFF on install at the moment, so this is required.

llvm/cmake/modules/AddLLVM.cmake

2357–2359

nit: let's update the comment

[qiongsiwu1]

Addressing review comment.

[qiongsiwu1]

llvm/cmake/modules/AddLLVM.cmake

2357–2359

Ah thanks for catching this David! Fixed.

[finagolfin]

Ah, sorry, I solicited feedback on the AIX changes on the original review, but got none. You will also want to undo the split of the AIX rpaths I put in above, moving everything to _install_rpath instead.

Also, do you mind removing Windows from this list? I'm fairly certain that has no effect, as the logic above returns if not an Apple, Unix, or AIX platform, but I simply put it in because of a previous LLVM commit message for 959dbd1761c that reported this same error on Windows, likely erroneously.

[qiongsiwu1]

Addressing review comments.

[qiongsiwu1]

In D148866#4287133, @buttaface wrote:

Ah, sorry, I solicited feedback on the AIX changes on the original review, but got none. You will also want to undo the split of the AIX rpaths I put in above, moving everything to _install_rpath instead.

Also, do you mind removing Windows from this list? I'm fairly certain that has no effect, as the logic above returns if not an Apple, Unix, or AIX platform, but I simply put it in because of a previous LLVM commit message for 959dbd1761c that reported this same error on Windows, likely erroneously.

Thanks for the feedback @buttaface !! Not a problem. The engineer pinged in the original patch is no longer working on AIX. The patch is updated.

[finagolfin]

Thanks, LGTM.

llvm/cmake/modules/AddLLVM.cmake

function(llvm_setup_rpath name)
   if (APPLE)
     set(_install_name_dir INSTALL_NAME_DIR "@rpath")
     set(_install_rpath "@loader_path/../lib${LLVM_LIBDIR_SUFFIX}" ${extra_libdir})
   elseif(${CMAKE_SYSTEM_NAME} MATCHES "AIX" AND BUILD_SHARED_LIBS)
     # $ORIGIN is not interpreted at link time by aix ld.
     # Since BUILD_SHARED_LIBS is only recommended for use by developers,
     # hardcode the rpath to build/install lib dir first in this mode.
     # FIXME: update this when there is better solution.
-    set(_build_rpath "${LLVM_LIBRARY_OUTPUT_INTDIR}" ${extra_libdir})
-    set(_install_rpath "${CMAKE_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+    set(_install_rpath "${LLVM_LIBRARY_OUTPUT_INTDIR}" "${CMAKE_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}" ${extra_libdir})
   elseif(UNIX)
     set(_build_rpath "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}" ${extra_libdir})
     set(_install_rpath "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}")
     if(${CMAKE_SYSTEM_NAME} MATCHES "(FreeBSD|DragonFly)")
       set_property(TARGET ${name} APPEND_STRING PROPERTY
                    LINK_FLAGS " -Wl,-z,origin ")
     endif()
     if(LLVM_LINKER_IS_GNULD)
       # $ORIGIN is not interpreted at link time by ld.bfd
       set_property(TARGET ${name} APPEND_STRING PROPERTY
                    LINK_FLAGS " -Wl,-rpath-link,${LLVM_LIBRARY_OUTPUT_INTDIR} ")
     endif()
   else()
     return()
   endif()
 
   # Enable BUILD_WITH_INSTALL_RPATH unless CMAKE_BUILD_RPATH is set and not
-  # building for macOS or Windows, as those two platforms seemingly require it.
+  # building for macOS or AIX, as those platforms seemingly require it.
+  # On AIX, the tool chain doesn't support modifying rpaths/libpaths for XCOFF
+  # on install at the moment, so BUILD_WITH_INSTALL_RPATH is required.
   if("${CMAKE_BUILD_RPATH}" STREQUAL "")
-    if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin|Windows")
+    if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin|AIX")
       set_property(TARGET ${name} PROPERTY BUILD_WITH_INSTALL_RPATH ON)
     else()
       set_property(TARGET ${name} APPEND PROPERTY BUILD_RPATH "${_build_rpath}")
     endif()
   endif()
 
   set_target_properties(${name} PROPERTIES
                         INSTALL_RPATH "${_install_rpath}"