[libcxx-commits] [PATCH] D111244: [libc++][AIX] Add scripts and config for building with the libcxx CI infrastructure

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Awesome! Thanks a lot for looking into adding AIX to the supported configurations. Once we have an entry in `buildkite-pipeline.yml` and bots running that configuration, we can even list the platform as being officially supported in the docs!



================
Comment at: libcxx/cmake/caches/AIX.cmake:3-5
+set(CMAKE_C_FLAGS "-D__LIBC_NO_CPP_MATH_OVERLOADS__" CACHE STRING "")
+set(CMAKE_CXX_FLAGS "-D__LIBC_NO_CPP_MATH_OVERLOADS__" CACHE STRING "")
+set(CMAKE_SHARED_LINKER_FLAGS "-Wl,-G -Wl,-bcdtors:all:-2147483548:s" CACHE STRING "")
----------------
😭

Is that all necessary for libc++ to work on top of AIX? If so, does that mean users have to specify those when building their own programs too? It's none of my business, but perhaps it'd be awesome if the Clang driver (and the C library for `__LIBC_NO_CPP_MATH_OVERLOADS__`) handled a bit more automatically, from a user perspective.


================
Comment at: libcxx/test/configs/llvm-libc++-shared.cfg.in:13
 COMPILER = "@CMAKE_CXX_COMPILER@"
 EXEC_ROOT = "@LIBCXX_BINARY_DIR@"
 CMAKE_OSX_SYSROOT = "@CMAKE_OSX_SYSROOT@"
----------------
Can you duplicate this file for AIX instead?

It'll seem backwards and a really bad solution, but after landing https://reviews.llvm.org/D111196, those shouldn't contain so much duplication. I need to write some documentation to explain the intent of those "from-scratch" config files, but essentially they should be as close as possible to containing a vanilla compiler invocation for building a Hello World program on a given platform. I find it really important to avoid adding logic to those files since that's what we did with the old `config.py` and that's how it became impossible to understand (and impossible to verify that we were running the test suite correctly).


================
Comment at: libcxx/utils/ci/run-buildbot:592
+    generate-cmake-aix -C "${MONOREPO_ROOT}/libcxx/cmake/caches/AIX.cmake" \
+                   -DLIBCXX_TEST_CONFIG="${MONOREPO_ROOT}/libcxx/test/configs/llvm-libc++-shared.cfg.in"
+    check-cxx-cxxabi
----------------
Nitpick, but let's line them up while we're at it! Also, we can now use relative paths (there were quite a few changes in that area lately, so you should rebase onto `main`).

The name of the config is just a suggestion.


================
Comment at: libcxxabi/test/lit.site.cfg.in:1
 @AUTO_GEN_COMMENT@
 
----------------
I'm going to suggest that you don't even modify this file at all. No need to support legacy testing configs, we're trying to move away from them. Instead, you could specify 

```
-DLIBCXXABI_TEST_CONFIG="ibm-libc++-shared.cfg.in"
```

in `run-buildbot` above and you'd never use the legacy config!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111244