[libc-commits] [PATCH] D74897: [libc] Add CMake script to check host cpu features

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:1
+set(ALL_CPU_FEATURES)
+
----------------
I had a hard time jumping through the various pieces here. WDYT of this suggestion:

Populate `ALL_CPU_FEATURES` first up as:

```
if(${LIBC_TARGET_MACHINE} MATCHES "x86|x86_64")
  set(ALL_CPU_FEATURES SSE SSE2 AVX AVX512)
endif()
```

Then, you can have just one function called `check_cpu_features` which populates `HOST_CPU_FEATURES` as a return value. You choose to use a helper function like `define_compiler_options_for_feature`:

```
function(define_compiler_options_for_feature feature)
  if(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
    string(TOLOWER ${feature} lowercase_feature)
    set(CPU_FEATURE_${name}_ENABLE_FLAG "-m${lowercase_feature}" PARENT_SCOPE)
    set(CPU_FEATURE_${name}_DISABLE_FLAG "-mno-${lowercase_feature}" PARENT_SCOPE)
  else()
    # In future, we can extend for other compilers.
    message(FATAL "Unkown compiler ${CMAKE_CXX_COMPILER_ID}.")
  endif()
endfunction()
```


================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:22
+
+if(${LIBC_TARGET_MACHINE} MATCHES "x86|x86_64")
+    define_cpu_feature_optimization(SSE CLANG_FLAG sse)
----------------
We shouldn't need this block.


================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:29
+
+set(HOST_CPU_FEATURES)
+function(check_cpu_features feature)
----------------
Does it need to be set here?


================
Comment at: libc/cmake/modules/LLVMLibCCheckCpuFeatures.cmake:51
+# HOST_CPU_FEATURES with a subset of ALL_CPU_FEATURES.
+foreach(feature IN LISTS ALL_CPU_FEATURES)
+    check_cpu_features(${feature})
----------------
We can push this `foreach` block into the above function and instead have a call to the function without any args? If you want to split, then may be call the above function `check_single_cpu_feature` which gives a yes/no answer, and the function `check_cpu_features` will call this single feature function and populate `AVAILABLE_CPU_FEATURES` as a return value. When cross-compiling, one will have to set `AVAILABLE_CPU_FEATURES` manually.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74897