[libcxx-commits] [PATCH] D125121: [runtimes] Generalize how we reorder projects

Fri May 6 16:53:54 PDT 2022

phosek added inline comments.

================
Comment at: runtimes/CMakeLists.txt:16
+
+# We order compiler-rt first so that other projects can have tests like `if(TARGET asan)`.
+# Similarly, we put libcxxabi before libcxx because libcxx may depend on targets created
----------------
I searched for `if(TARGET asan)` and we don't seem to use that anywhere so I'm not sure if this comment is still true. It's possible that this is outdated and we no longer need to do this.

compiler-rt [can depend on libcxx](https://github.com/llvm/llvm-project/blob/f827ee671f5440a7514d071abbfec0556119a34f/compiler-rt/CMakeLists.txt#L609) so I think it would make more sense to put `compiler-rt` after `libcxx`, we just need to check that nothing breaks if we do that.

================
Comment at: runtimes/CMakeLists.txt:19
+# by libcxxabi.
+set(LLVM_ALL_RUNTIMES "compiler-rt;libcxxabi;libcxx;libunwind;libc;openmp")
 set(LLVM_ENABLE_RUNTIMES "" CACHE STRING
----------------
I think that `libc` and `libunwind` should come before `libcxxabi` and `libcxx` since that matches how these libraries are layered.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125121