[PATCH] D88932: [runtimes][compiler-rt] Add optional runtimes step to build crt up front

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: compiler-rt/cmake/config-ix.cmake:670
 
-if (CRT_SUPPORTED_ARCH AND OS_NAME MATCHES "Linux" AND NOT LLVM_USE_SANITIZER)
   set(COMPILER_RT_HAS_CRT TRUE)
----------------
What does setting this to `TRUE` more often do?


================
Comment at: compiler-rt/cmake/crt-config-ix.cmake:25-41
+set(ARM64 aarch64)
+set(ARM32 arm armhf armv6m armv7m armv7em armv7 armv7s armv7k)
+set(HEXAGON hexagon)
+set(X86 i386)
+set(X86_64 x86_64)
+set(MIPS32 mips mipsel)
+set(MIPS64 mips64 mips64el)
----------------
My guess is that the unreferenced lists here are going to become unmaintained lists very quickly. Also, what is the scope associated with these? Are we sure that these are not overriding other definitions before their corresponding use?


================
Comment at: compiler-rt/cmake/crt-config-ix.cmake:47
+
+# If we're not building the crt standalone, just rely on the  tests in
+# config-ix.cmake to tell us what to build. Otherwise we need to do some leg
----------------
Nit: re: extra space.


================
Comment at: compiler-rt/cmake/crt-config-ix.cmake:48
+# If we're not building the crt standalone, just rely on the  tests in
+# config-ix.cmake to tell us what to build. Otherwise we need to do some leg
+# work here...
----------------
Nit: Comma.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88932