[PATCH] D83866: [SystemZ/ZOS] Additions to the build system.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added a subscriber: daltenty.
hubert.reinterpretcast added inline comments.


================
Comment at: llvm/CMakeLists.txt:410
+if (CMAKE_SYSTEM_NAME MATCHES "OS390")
+  option(LLVM_ENABLE_PIC "Build Position-Independent Code" OFF)
+else()
----------------
Is it actually true that the resulting code is not position-independent? I thought that all XPLINK code is DLL-enabled. The outcome where the resulting code is position-independent (without the need to specify additional options) should be expressed differently from what is done here.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:252
       elseif(NOT WIN32 AND NOT LLVM_LINKER_IS_GOLD AND
-             NOT ${CMAKE_SYSTEM_NAME} MATCHES "OpenBSD|AIX")
+             NOT CMAKE_SYSTEM_NAME MATCHES "OpenBSD|AIX|OS390")
         # Object files are compiled with -ffunction-data-sections.
----------------
The various "strip" binder options are not the default, so there is a TODO here to enable the removal of the unneeded portions of the input to the linking process (which should help in terms of binary size).


================
Comment at: llvm/cmake/modules/GetHostTriple.cmake:19
+    set( value "s390x-ibm-zos" )
   elseif( CMAKE_HOST_SYSTEM_NAME STREQUAL AIX )
     # We defer to dynamic detection of the host AIX version.
----------------
I think `CMAKE_SYSTEM_NAME` (used above for z/OS) is right: The "host" of the resulting build is the target of this CMake build. @daltenty, if you agree, I think a small patch would make sense (likely to create a merge conflict with this one though)...


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:1010
+if(CMAKE_SYSTEM_NAME MATCHES "OS390" AND LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)
+  message(FATAL_ERROR "Generating export from the static libraries is not supported on z/OS")
+endif()
----------------
What is the nature of this limitation? "Not supported" messages tend to confuse, because someone wanting to solve the issue (other than not using whatever is "unsupported") needs some rationale in order to make progress.

Indeed, the text of the message speaks to a limitation to the implementation strategy used by the build and does not directly say much about there being a limitation in achieving the outcome that the option is intended to produce.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83866