[libc-commits] [PATCH] D94692: [libc] Allow customization of memcpy via flags.

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Thu Jan 14 09:10:38 PST 2021


sivachandra accepted this revision.
sivachandra added a comment.
This revision is now accepted and ready to land.

Few inline nits.



================
Comment at: libc/src/__support/common.h:42
+#define LLVM_LIBC_IS_DEFINED__EVAL_AND_STRINGIZE(s) #s
+namespace details {
+// C++14 version of compile time string equality.
----------------
In other places, we have named such namespaces as `internal`.


================
Comment at: libc/src/__support/common.h:45
+// Once we require C++17 we can leverage std::string_view. e.g.
+// std::string_view(lhs) == rhs;
+constexpr bool same_string(char const *lhs, char const *rhs) {
----------------
Just pointing out, we cannot use the C++ standard library. So, this comment should either be removed or updated suitably.


================
Comment at: libc/src/string/x86/memcpy.cpp:64
                        size_t count) {
-  if (count == 0)
-    return;
-  if (count == 1)
-    return CopyBlock<1>(dst, src);
-  if (count == 2)
-    return CopyBlock<2>(dst, src);
-  if (count == 3)
-    return CopyBlock<3>(dst, src);
-  if (count == 4)
-    return CopyBlock<4>(dst, src);
-  if (count < 8)
-    return CopyBlockOverlap<4>(dst, src, count);
-  if (count < 16)
-    return CopyBlockOverlap<8>(dst, src, count);
-  if (count < 32)
-    return CopyBlockOverlap<16>(dst, src, count);
-  if (count < 64)
-    return CopyBlockOverlap<32>(dst, src, count);
-  if (count < 128)
-    return CopyBlockOverlap<64>(dst, src, count);
-#if defined(__AVX__)
-  if (count < 256)
-    return CopyBlockOverlap<128>(dst, src, count);
-#endif
-  // kRepMovsBSize == -1 : Only CopyAligned is used.
-  // kRepMovsBSize ==  0 : Only RepMovsb is used.
-  // else CopyAligned is used to to kRepMovsBSize and then RepMovsb.
-  constexpr size_t kRepMovsBSize = -1;
-  if (count <= kRepMovsBSize)
-    return CopyAlignedBlocks<BEST_SIZE>(dst, src, count);
-  return CopyRepMovsb(dst, src, count);
+  if (kUseOnlyRepMovsb) {
+    return CopyRepMovsb(dst, src, count);
----------------
Can we use the early return pattern for this as well without the else?

```
   if (kUseOnlyRepMovSb)
     return CopyRepMovsb(...);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94692



More information about the libc-commits mailing list