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

[Alex Brachet via Phabricator via libc-commits]

abrachet added a comment.

As this patch gets closer to landing I think it would make sense to update the description :)



================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:10
+list(SORT ALL_CPU_FEATURES)
+message(STATUS "Available Cpu Features: ${ALL_CPU_FEATURES}")
+
----------------
Is it necessary to print this?


================
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
+#endif
----------------
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.


================
Comment at: libc/cmake/modules/cpu_features/check_cpu_features.cpp.in:19-20
+    if (i)
+      putc(';', stdout);
+    fputs(strings[i], stdout);
+  }
----------------
Why not `putchar` and `puts`?


================
Comment at: libc/test/src/string/CMakeLists.txt:32
+  if(can_run)
+    compute_flags(flags MARCH native)
+    add_libc_unittest(
----------------
Would you mind explaining this? It seems like ${flags} will just be -march=native, and the work above to find flags gets ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74397