[PATCH] D60253: [gn] Support for building runtimes

[Petr Hosek via Phabricator via llvm-commits]

phosek marked 3 inline comments as done.
phosek added inline comments.


================
Comment at: llvm/utils/gn/secondary/clang/runtimes.gni:4
+
+runtimes_dir = "$clang_resource_dir/$llvm_target_triple/lib"
----------------
pcc wrote:
> In http://llvm-cs.pcc.me.uk/utils/gn/secondary/compiler-rt/target.gni we have crt_current_out_dir which is basically the same as this.
> 
> IIUC this is the "new" style of runtime paths which is currently only used by Fuchsia. Should the code there be changed to do "if (is_fuchsia) use the new-style paths else use the old-style paths"?
The difference is that `crt_current_out_dir` applies only to compiler-rt while `runtimes_dir` applies to libunwind/libc++abi/libc++ and soon omp as well.

I don't think it's Fuchsia specific, it's true that we're the main user, but as discussed in the [RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)](http://lists.llvm.org/pipermail/cfe-dev/2019-February/061307.html), there seems to be an agreement about making that the default, this layout is also used by other clients, e.g. Google internally. I'm fine putting it behind an argument, e.g. `clang_enable_per_target_runtime_dir` the same as we do in the CMake build.


================
Comment at: llvm/utils/gn/secondary/libcxx/src/BUILD.gn:4
+
+declare_args() {
+  # Build libc++ with definitions for operator new/delete.
----------------
pcc wrote:
> Do you really need all of these arguments? I would recommend only implementing the default behavior for most of them except for the ones that you actually need.
> 
> (Same comment elsewhere.)
We need these for Fuchsia and/or libFuzzer (which is also needed for Fuchsia).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60253