[libc-commits] [PATCH] D148756: [libc] Add rule named `add_libc_hermetic_test` which adds a hermetic test.

[Joseph Huber via Phabricator via libc-commits]

jhuber6 added a comment.

Thanks a lot for this work, I think it's getting us pretty close. Getting this to build for the GPU required adding the `UnitTest` and `src` directories in the CMake. Some other issues I've had when trying to build for the GPU.

1. Including `stdlib.h` in `src/__support/CPP/string.h` causes errors when they include the system headers. Presumably this is because we aren't providing this as a header in the GPU build yet?
2. The calls to `::free` and `::realloc` in `src/__support/CPP/string.h` did not compile when I tried, they were undefined in the module.
3. I got an LLVM crash when compiling the whole project targeting AMDGPU in LTO mode. This is the recommended way as AMDGPU doesn't have a true "ABI". Turning off LTO hides the crash but is a little more iffy on whether or not it's correct since there's no defined ABI for cross-TU calls.
4. My build was not picking up `libc.src.__support.OSUtil.osutil` in the final test for some reason, added it to the hermetic test and it worked.
5. We do not have `__cxa_atexit` defined
6. The `UnitTest` libraries need to have the explicit `--target=` and `-mcpu=` options passed in all cases to match the source.
7. After hacking around all of the above, the test built and runs with a "No tests run." message. I'm assuming this is because we are not handling the global constructors.

I'd appreciate some pointers on how to fix some of the above on the GPU side, I'm not as intimately familiar with the `libc` internals for registering constructors / exit functions.



================
Comment at: libc/cmake/modules/LLVMLibCTestRules.cmake:562
+    "HERMETIC_TEST"
+    "" # No optional arguments
+    "SUITE" # Single value arguments
----------------
Optional `LOADER_ARGS` here needed for the GPU.


================
Comment at: libc/cmake/modules/LLVMLibCTestRules.cmake:673
+    COMMAND ${HERMETIC_TEST_ENV}
+            $<$<BOOL:${LIBC_TARGET_ARCHITECTURE_IS_GPU}>:${gpu_loader_exe}>
+            $<TARGET_FILE:${fq_build_target_name}> ${HERMETIC_TEST_ARGS}
----------------
jhuber6 wrote:
> Small change here just today that adds `LOADER_ARGS` to support calling tests with multiple threads on the GPU.



================
Comment at: libc/test/UnitTest/CMakeLists.txt:14
 endif()
 
 add_library(
----------------
The GPU support will require overloading the triple and architecture for each one of these. So the above should work applied as compile options for each `add_library` under here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148756