ldionne added a comment.

I agree the design is clean from the "what we ship" perspective. I really like 
the idea of shipping the whole library in a generic spot and only having one 
file embody all the target specific configuration of the library. I think it's 
a very elegant way to handle the configuration of libc++, and it will have 
positive effects beyond your intended use case like giving us a place and a way 
to push platform-specific hacks to.

Where this really stinks IMO is in the way we build the library. I don't want 
us to have a separate `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` setting and a 
different mode for building libc++. This is at the heart of many discussions 
we've been having lately. Basically, the way I do things:

1. Build libc++ for one target using the most straightforward possible CMake 
settings
2. Then, post-process that by moving headers around, creating universal 
binaries, etc, all of it from a separate script.

That's how I set up the libc++ build at Apple because I didn't want to add too 
many platform specific things in the CMake. Otherwise we'd have e.g. a mode for 
building for many architectures (`x86_64` and `arm64`) at once on Apple 
platforms and creating a universal dylib from that. That's not the way CMake is 
intended to be used unless I fundamentally missed something. Also note that it 
would be much better if CMake had proper support for multi-arch, but I'm not 
aware of anything like that.

On the contrary, your approach (please correct me if I'm mis-speaking) is to 
make the CMake work out of the box exactly as you want it, even if that means 
adding complexity. This has the nice benefit that everything is done in a 
single place, however it forces us to go against the way CMake was designed in 
a few places, for example when it comes to building the library for several 
platforms.

My only push back on this patch, and on several patches in the same vein, is 
about this fundamental difference in approach that we have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89013

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to