[PATCH] D74256: [AIX] Change host detection to return correct 32/64-bit triple

Wed Feb 12 12:06:31 PST 2020

stevewan added inline comments.

================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:174
-    set(CMAKE_CXX_ARCHIVE_FINISH "<CMAKE_RANLIB> -X64 <TARGET>")
-  endif()
   # -fPIC does not enable the large code model for GCC on AIX but does for XL.
----------------
daltenty wrote:
> stevewan wrote:
> > Is this change somewhat out of scope here and fits better in a separate patch?
> The reason I'd included in this patch is it affects your ability to test this change. CMake won't set CMAKE_SIZEOF_VOID_P to the 64-bit case unless you have OBJECT_MODE=64 set, but these options override the OBJECT_MODE setting.
> 
> Maybe the best option is to split this patch and land this bit first.
How's the value of `LLVM_BUILD_32_BITS` determined? I found in llvm/CMakeLists.txt that,
```
if( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32 )
  # TODO: support other platforms and toolchains.
  option(LLVM_BUILD_32_BITS "Build 32 bits executables and libraries." OFF)
endif()
```
which suggests that `LLVM_BUILD_32_BITS` depends on `CMAKE_SIZEOF_VOID_P`, but then that means there is a circular dependency? Did I misunderstand something?

Given this piece of code is related to topic, I'm fine with either spliting or keeping it. If we do decide to include it, we'll need to expand the description of this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74256