[libc-commits] [PATCH] D139939: [libc] Make string functions buildable with GCC

[Guillaume Chatelet via Phabricator via libc-commits]

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


================
Comment at: libc/src/string/memory_utils/bcmp_implementations.h:103
     return generic::Bcmp<8>::head_tail(p1, p2, count);
-  if constexpr (x86::kAvx512BW)
-    return inline_bcmp_x86_avx512bw_gt16(p1, p2, count);
----------------
sivachandra wrote:
> What was the problem with these constexpr conditionals? Because the `inline_*` functions are now potentially excluded by the preprocessor, they could be invisible at compile time?
avx512 functions requires a specific ABI. When compiling `op_x86.h` without avx512 support, GCC complains about these avx512 specific functions not having the right ABI -> I had to selectively exclude them using the preprocessor. Now these functions do not exist anymore when not compiling for avx512, so we cannot refer to them in the `XXX_implementations_h` files.

Using `if constexpr` would fail since the code still needs to be valid (even if disabled) so I had to use the preprocessor here as well.

This is far from ideal, it adds a lot of visual clutter.

Another option would be to disable the body of the function in `op_x86.h` instead of the function itself. So the function would still exist and could be referred to through `if constexpr` constructs.
I'll give it a try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139939