[PATCH] D143332: [CMake] Unify compiler-rt flag variables

Wed Feb 8 13:30:56 PST 2023

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

Generally LGTM. I didn't check for potential mismatches across the patch, if there are other instances of the variables that need to be fixed, or if there are other variables that should be renamed though.

I did notice some inconsistencies and missing cases within the patch context though.

================
Comment at: compiler-rt/cmake/config-ix.cmake:196
 # Linker flags.
 llvm_check_compiler_linker_flag(C "-Wl,-z,text" COMPILER_RT_HAS_Z_TEXT)
+llvm_check_compiler_linker_flag(C "-fuse-ld=lld" CXX_SUPPORTS_FUSE_LD_LLD_FLAG)
----------------
What about this flag?

================
Comment at: compiler-rt/cmake/config-ix.cmake:197
 llvm_check_compiler_linker_flag(C "-Wl,-z,text" COMPILER_RT_HAS_Z_TEXT)
-llvm_check_compiler_linker_flag(C "-fuse-ld=lld" COMPILER_RT_HAS_FUSE_LD_LLD_FLAG)
+llvm_check_compiler_linker_flag(C "-fuse-ld=lld" CXX_SUPPORTS_FUSE_LD_LLD_FLAG)

----------------
This test is for `C`, not `CXX`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143332