Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] Enable missing memory tests on the GPU #68111

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Oct 3, 2023

Summary:
There were a few tests that weren't enabled on the GPU. This is because
the logic caused them to be skipped as we don't use CPU featured on the
host. This also disables the logic making multiple versions of the
memory functions.

Summary:
There were a few tests that weren't enabled on the GPU. This is because
the logic caused them to be skipped as we don't use CPU featured on the
host. This also disables the logic making multiple versions of the
memory functions.
@llvmbot
Copy link

llvmbot commented Oct 3, 2023

@llvm/pr-subscribers-libc

Changes

Summary:
There were a few tests that weren't enabled on the GPU. This is because
the logic caused them to be skipped as we don't use CPU featured on the
host. This also disables the logic making multiple versions of the
memory functions.


Full diff: https://github.com/llvm/llvm-project/pull/68111.diff

3 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake (-4)
  • (modified) libc/src/string/CMakeLists.txt (+12)
  • (modified) libc/test/src/string/CMakeLists.txt (+4-1)
diff --git a/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake b/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
index 0b522318aaa129e..73b249374a0667c 100644
--- a/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
+++ b/libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake
@@ -22,10 +22,6 @@ list(SORT ALL_CPU_FEATURES)
 #   <list of cpu features>
 # )
 function(cpu_supports output_var features)
-  if(LIBC_TARGET_ARCHITECTURE_IS_GPU)
-    unset(${output_var} PARENT_SCOPE)
-    return()
-  endif()
   _intersection(var "${LIBC_CPU_FEATURES}" "${features}")
   if("${var}" STREQUAL "${features}")
     set(${output_var} TRUE PARENT_SCOPE)
diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index f2e5654d03ccece..67675b682081c67 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -498,6 +498,8 @@ if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
   add_bcmp(bcmp_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512BW)
   add_bcmp(bcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bcmp(bcmp)
+elseif(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  add_bcmp(bcmp)
 else()
   add_bcmp(bcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bcmp(bcmp)
@@ -525,6 +527,8 @@ if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
   add_bzero(bzero_x86_64_opt_avx512 COMPILE_OPTIONS -march=skylake-avx512 REQUIRE AVX512F)
   add_bzero(bzero_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bzero(bzero)
+elseif(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  add_bzero(bzero)
 else()
   add_bzero(bzero_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_bzero(bzero)
@@ -555,6 +559,8 @@ if(${LIBC_TARGET_ARCHITECTURE_IS_X86})
 elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   add_memcmp(memcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memcmp(memcmp)
+elseif(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  add_memcmp(memcmp)
 else()
   add_memcmp(memcmp_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memcmp(memcmp)
@@ -589,6 +595,8 @@ elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   add_memcpy(memcpy_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE}
                                       MLLVM_COMPILE_OPTIONS "-tail-merge-threshold=0")
   add_memcpy(memcpy                   MLLVM_COMPILE_OPTIONS "-tail-merge-threshold=0")
+elseif(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  add_memcpy(memcpy)
 else()
   add_memcpy(memcpy_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memcpy(memcpy)
@@ -621,6 +629,8 @@ elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   add_memmove(memmove_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE}
                                         MLLVM_COMPILE_OPTIONS "-tail-merge-threshold=0")
   add_memmove(memmove                   MLLVM_COMPILE_OPTIONS "-tail-merge-threshold=0")
+elseif(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  add_memmove(memmove)
 else()
   add_memmove(memmove_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memmove(memmove)
@@ -653,6 +663,8 @@ elseif(${LIBC_TARGET_ARCHITECTURE_IS_AARCH64})
   add_memset(memset_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE}
                                       MLLVM_COMPILE_OPTIONS "-tail-merge-threshold=0")
   add_memset(memset                   MLLVM_COMPILE_OPTIONS "-tail-merge-threshold=0")
+elseif(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  add_memset(memset)
 else()
   add_memset(memset_opt_host          COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_NATIVE})
   add_memset(memset)
diff --git a/libc/test/src/string/CMakeLists.txt b/libc/test/src/string/CMakeLists.txt
index b90da43f9a7b94c..6088289532d771d 100644
--- a/libc/test/src/string/CMakeLists.txt
+++ b/libc/test/src/string/CMakeLists.txt
@@ -425,8 +425,11 @@ function(add_libc_multi_impl_test name)
     get_target_property(required_cpu_features ${fq_config_name} REQUIRE_CPU_FEATURES)
     cpu_supports(can_run "${required_cpu_features}")
     if(can_run)
+      string(FIND ${fq_config_name} "." last_dot_loc REVERSE)
+      math(EXPR name_loc "${last_dot_loc} + 1")
+      string(SUBSTRING ${fq_config_name} ${name_loc} -1 target_name)
       add_libc_test(
-        ${fq_config_name}_test
+        ${target_name}_test
         SUITE
           libc-string-tests
         COMPILE_OPTIONS

@jhuber6
Copy link
Contributor Author

jhuber6 commented Oct 5, 2023

I've landed the patch that replaces the memory functions with builtins. This patch enables us actually testing them. The only significant change is that I make a more usable name for the test target, it used to be like libc.test.src.string.libc.src.string.memcpy_test and now it's libc.test.src.string.memcpy_test.

Copy link
Collaborator

@yxsamliu yxsamliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@jhuber6 jhuber6 merged commit 4cb6c1c into llvm:main Oct 6, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants