[libc-commits] [PATCH] D74397: [libc] Adding memcpy implementation for x86_64

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Mar 16 09:48:08 PDT 2020

sivachandra marked an inline comment as done.
sivachandra added inline comments.

Comment at: libc/src/string/CMakeLists.txt:83
+add_memcpy(memcpy MARCH native)
It seems like this will not ensure the best flags. Is that the intention? If so, why?

Comment at: libc/src/string/x86_64/CMakeLists.txt:1
+add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_none" REJECT "${ALL_CPU_FEATURES}")
+add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_sse" REQUIRE "SSE" REJECT "SSE2")
These listings are already under `x86_64`. So, do we need to use `${LIBC_TARGET_MACHINE}`?

Comment at: libc/src/string/x86_64/CMakeLists.txt:4
+add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_avx" REQUIRE "AVX" REJECT "AVX2")
+add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_avx512f" REQUIRE "AVX512F")
It would be nice if a descriptive error message is shown if one tries to build a target which isn't supported on their machine. ISTM that it is not the case. If indeed so, we can do it in a later round I think.

Comment at: libc/test/src/string/CMakeLists.txt:32
+  if(can_run)
+    compute_flags(flags MARCH native)
+    add_libc_unittest(
gchatelet wrote:
> abrachet wrote:
> > Would you mind explaining this? It seems like ${flags} will just be -march=native, and the work above to find flags gets ignored.
> Actually there's no need for flags here, the implementation has already been compiled with the correct flags. The test itself doesn't need them.
Same question for me as well. I left a related comment at a different place above.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list