[libc-commits] [PATCH] D142108: [libc][NFC] Detect host CPU features using try_compile instead of try_run.

[Siva Chandra via Phabricator via libc-commits]

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

Mostly LGTM but a few minor comments.

I don't see a `CMakeLists.txt` change and a corresponding Bazel change for the new header file.

In D142108#4065441 <https://reviews.llvm.org/D142108#4065441>, @gchatelet wrote:

> - Do we want to merge "architectures.h" and "cpu_features.h" since they usually are used together?

I like the separation.

> - Do we want to autogenerate the C files since they are not quite minimal?

I think the explicit listing is probably a better approach. As this get more complicated, the CMake scripting will get more complicated. I prefer simpler CMake scripting over increased verbosity/repetitiveness.



================
Comment at: libc/cmake/modules/cpu_features/check_AVX2.c:1
+#include "src/__support/cpu_features.h"
+
----------------
Nit: These checker sources are including a `.h` which is marked `C++`. Also, we need a C++ compiler anyway for the building the libc anyway. So, why not make these files have a `.cpp` extension?


================
Comment at: libc/src/__support/cpu_features.h:8
+//===----------------------------------------------------------------------===//
+// This file target cpu features by introspecting compiler enabled preprocessor
+// definitions.
----------------
This comment seems incomplete; Perhaps, `"This file **lists** target CPU features ..."` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142108