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

Guillaume Chatelet via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Mar 16 09:48:06 PDT 2020

gchatelet added inline comments.

Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:10
+message(STATUS "Available Cpu Features: ${ALL_CPU_FEATURES}")
abrachet wrote:
> Is it necessary to print this?
Not really.

Comment at: libc/cmake/modules/cpu_features/check_cpu_features.cpp.in:4-10
+// Handle MSVC SSE
+#if (defined(_M_AMD64) || defined(_M_X64) || (_M_IX86_FP == 2))
+#define __SSE2__ 1
+#define __SSE__ 1
+#elif _M_IX86_FP == 1
+#define __SSE__ 1
abrachet wrote:
> Is it necessary to define these? Does DEFINITIONS rely on the definitions of macros? Also I think handling MSVC here is strange when `compute_flags` does not currently.
Right, it's probably better to defer anything related to MSVC at that point.
Also I'll add documentation to this file because it's a bit obscure.

Comment at: libc/cmake/modules/cpu_features/check_cpu_features.cpp.in:19-20
+    if (i)
+      putc(';', stdout);
+    fputs(strings[i], stdout);
+  }
abrachet wrote:
> Why not `putchar` and `puts`?
I used `putchar` for a single character but I can't use `puts` since it automatically appends a line feed.

Comment at: libc/test/src/string/CMakeLists.txt:32
+  if(can_run)
+    compute_flags(flags MARCH native)
+    add_libc_unittest(
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.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list