[libc][cmake] Do not overwrite SKIP_FLAG_EXPANSION_*.

[lntue]

So that users can set these manually if needed.

[llvmbot]

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

So that users can set these manually if needed.

---

Full diff: https://github.com/llvm/llvm-project/pull/125762.diff

1 Files Affected:

• (modified) libc/cmake/modules/LLVMLibCFlagRules.cmake (+14-8)

diff --git a/libc/cmake/modules/LLVMLibCFlagRules.cmake b/libc/cmake/modules/LLVMLibCFlagRules.cmake
index e5b0e249c90676..7d5e73c2f12148 100644
--- a/libc/cmake/modules/LLVMLibCFlagRules.cmake
+++ b/libc/cmake/modules/LLVMLibCFlagRules.cmake
@@ -268,23 +268,29 @@ set(EXPLICIT_SIMD_OPT_FLAG "EXPLICIT_SIMD_OPT")
 set(MISC_MATH_BASIC_OPS_OPT_FLAG "MISC_MATH_BASIC_OPS_OPT")
 
 # Skip FMA_OPT flag for targets that don't support fma.
-if(NOT((LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "FMA")) OR
-       LIBC_TARGET_ARCHITECTURE_IS_RISCV64))
-  set(SKIP_FLAG_EXPANSION_FMA_OPT TRUE)
+if(NOT DEFINED SKIP_FLAG_EXPANSION_FMA_OPT)
+  if(NOT((LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "FMA")) OR
+        LIBC_TARGET_ARCHITECTURE_IS_RISCV64))
+    set(SKIP_FLAG_EXPANSION_FMA_OPT TRUE)
+  endif()
 endif()
 
 # Skip EXPLICIT_SIMD_OPT flag for targets that don't support SSE2.
 # Note: one may want to revisit it if they want to control other explicit SIMD
-if(NOT(LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "SSE2")))
-  set(SKIP_FLAG_EXPANSION_EXPLICIT_SIMD_OPT TRUE)
+if(NOT DEFINED SKIP_FLAG_EXPANSION_EXPLICIT_SIMD_OPT)
+  if(NOT(LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "SSE2")))
+    set(SKIP_FLAG_EXPANSION_EXPLICIT_SIMD_OPT TRUE)
+  endif()
 endif()
 
 # Skip ROUND_OPT flag for targets that don't support rounding instructions. On
 # x86, these are SSE4.1 instructions, but we already had code to check for
 # SSE4.2 support.
-if(NOT((LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "SSE4_2")) OR
-       LIBC_TARGET_ARCHITECTURE_IS_AARCH64 OR LIBC_TARGET_OS_IS_GPU))
-  set(SKIP_FLAG_EXPANSION_ROUND_OPT TRUE)
+if(NOT DEFINED SKIP_FLAG_EXPANSION_ROUND_OPT)
+  if(NOT((LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "SSE4_2")) OR
+        LIBC_TARGET_ARCHITECTURE_IS_AARCH64 OR LIBC_TARGET_OS_IS_GPU))
+    set(SKIP_FLAG_EXPANSION_ROUND_OPT TRUE)
+  endif()
 endif()
 
 # Choose whether time_t is 32- or 64-bit, based on target architecture

[nickdesaulniers]

I thought that's what FORCE did for set statements?

https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry

i.e., if FORCE wasn't set, set would not re-set a value when it was specified by a user?

I don't care much either way, but perhaps I misremember what FORCE does. Because we don't use FORCE for these set statements, I would have thought we already do not overwrite these.

perhaps @petrhosek can clarify before you land this?

[llvm-ci]

LLVM Buildbot has detected a new failure on builder premerge-monolithic-linux running on premerge-linux-1 while building libc at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/22033

Here is the relevant piece of the build log for the reference

Step 7 (test-build-unified-tree-check-all) failure: test (failure)
...
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/bool.cpp (94129 of 98148)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/bounds.cpp (94130 of 98148)
UNSUPPORTED: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/no-interception.cpp (94131 of 98148)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/usub-overflow.cpp (94132 of 98148)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/uincdec-overflow.cpp (94133 of 98148)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/deduplication.cpp (94134 of 98148)
UNSUPPORTED: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/objc-cast.m (94135 of 98148)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/enum.cpp (94136 of 98148)
PASS: UBSan-MemorySanitizer-x86_64 :: TestCases/Misc/local_bounds.cpp (94137 of 98148)
TIMEOUT: MLIR :: Examples/standalone/test.toy (94138 of 98148)
******************** TEST 'MLIR :: Examples/standalone/test.toy' FAILED ********************
Exit Code: 1
Timeout: Reached timeout of 60 seconds

Command Output (stdout):
--
# RUN: at line 1
"/etc/cmake/bin/cmake" "/build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone" -G "Ninja"  -DCMAKE_CXX_COMPILER=/usr/bin/clang++  -DCMAKE_C_COMPILER=/usr/bin/clang   -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir  -DLLVM_USE_LINKER=lld  -DPython3_EXECUTABLE="/usr/bin/python3.10"
# executed command: /etc/cmake/bin/cmake /build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone -G Ninja -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld -DPython3_EXECUTABLE=/usr/bin/python3.10
# .---command stdout------------
# | -- The CXX compiler identification is Clang 16.0.6
# | -- The C compiler identification is Clang 16.0.6
# | -- Detecting CXX compiler ABI info
# | -- Detecting CXX compiler ABI info - done
# | -- Check for working CXX compiler: /usr/bin/clang++ - skipped
# | -- Detecting CXX compile features
# | -- Detecting CXX compile features - done
# | -- Detecting C compiler ABI info
# | -- Detecting C compiler ABI info - done
# | -- Check for working C compiler: /usr/bin/clang - skipped
# | -- Detecting C compile features
# | -- Detecting C compile features - done
# | -- Looking for histedit.h
# | -- Looking for histedit.h - found
# | -- Found LibEdit: /usr/include (found version "2.11") 
# | -- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.11") 
# | -- Found LibXml2: /usr/lib/x86_64-linux-gnu/libxml2.so (found version "2.9.13") 
# | -- Using MLIRConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir
# | -- Using LLVMConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/llvm
# | -- Linker detection: unknown
# | -- Performing Test LLVM_LIBSTDCXX_MIN
# | -- Performing Test LLVM_LIBSTDCXX_MIN - Success
# | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR
# | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR - Success
# | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER
# | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER - Success
# | -- Performing Test C_SUPPORTS_FPIC
# | -- Performing Test C_SUPPORTS_FPIC - Success
# | -- Performing Test CXX_SUPPORTS_FPIC

[petrhosek]

I thought that's what FORCE did for set statements?

https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry

i.e., if FORCE wasn't set, set would not re-set a value when it was specified by a user?

I don't care much either way, but perhaps I misremember what FORCE does. Because we don't use FORCE for these set statements, I would have thought we already do not overwrite these.

perhaps @petrhosek can clarify before you land this?

FORCE is used to overwrite a cached variable value but that's not what we're trying to do here.

[petrhosek]

What's the motivation for this change? Why would users want to enable these features even when they aren't supported?