[PATCH] D74256: [AIX] Improve 32/64-bit build configuration

[Steven Wan via Phabricator via llvm-commits]

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:
> > 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.
> I appears to be user configurable setting in you want to build a 32-bit version on a 64-bit host (which is why it only gets enabled if CMAKE_SIZEOF_VOID_P is 8). This option is probably not relevant on AIX (since you can just configure with CMake in 32-bit or 64-bit object modes and things should work as normal). 
> 
> We should probably not show this option on AIX either same as WIN32 (in many ways the AIX CMake setup is most similar to WIN32).
> 
> I will broaden the patch description, it makes sense to include this all as a single change
Yea, I agree that we should drop this option for AIX as it made the way of setting the object mode unnecessarily complex (more than one way of setting up the same thing), and introduced ambiguity (one approach would interfere with the other).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74256