[libc-commits] [PATCH] D149517: [libc] Enable running libc unit tests on AMDGPU

[Joseph Huber via Phabricator via libc-commits]

jhuber6 added inline comments.


================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:47
 
+if(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  return()
----------------
sivachandra wrote:
> Why was this moved?
Previously it prevented the above functions from being defined which are queried in the test setup.


================
Comment at: libc/cmake/modules/LLVMLibCTestRules.cmake:74
 function(create_libc_unittest fq_target_name)
-  if(NOT LLVM_INCLUDE_TESTS)
+  if(NOT LLVM_INCLUDE_TESTS OR NOT LIBC_ENABLE_UNITTESTS)
     return()
----------------
sivachandra wrote:
> Do we ever want to run unit tests the GPU? If no, then I think the right thing to do is to convert the unit test targets to `add_libc_test` targets which will then do the right thing.
I mostly did this because it was much less noise than converting everything to the new format. Since I figured that would be dependent on the rest of the `libc` project contributors to transition to the new test framework. This was the easy stopgap.


================
Comment at: libc/test/src/CMakeLists.txt:13
+    return()
+  endif()
+
----------------
sivachandra wrote:
> Why do you need this? If you don't include math tests in entrypoints.txt, this will not become active anyway, correct?
It was active when I tested it, I don't have any math entrypoints in the GPU entrypoint file. It built fine, but it spammed like 30 of these warnings about MPFR into the terminal so I just wanted to silence that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149517