ldionne added inline comments.
================ Comment at: README.md:71 + mlir, openmp, polly, or pstl. ``LLVM_ENABLE_RUNTIMES`` can include any of + libcxx, libcxxabi, libunwind, or compiler-rt. ---------------- tianshilei1992 wrote: > FWIW, OpenMP can be put into either of them. In OpenMP doc > (https://openmp.llvm.org/SupportAndFAQ.html#q-how-to-build-an-openmp-gpu-offload-capable-compiler), > we recommend to use `LLVM_ENABLE_RUNTIMES` to build OpenMP if users would > like to use offloading features. Thanks for the clarification, I tweaked the documentation a bit. ================ Comment at: bolt/docs/OptimizingClang.md:228-236 $ CPATH=${TOPLEV}/stage1/install/bin/ -$ cmake -G Ninja ${TOPLEV}/llvm-project/llvm -DLLVM_TARGETS_TO_BUILD=X86 \ +$ cmake -G Ninja -S ${TOPLEV}/llvm-project/llvm -B ${TOPLEV}/stage2-prof-gen \ + -DLLVM_TARGETS_TO_BUILD=X86 \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_C_COMPILER=$CPATH/clang -DCMAKE_CXX_COMPILER=$CPATH/clang++ \ -DLLVM_ENABLE_PROJECTS="clang;lld" \ -DLLVM_USE_LINKER=lld -DLLVM_BUILD_INSTRUMENTED=ON \ ---------------- Quuxplusone wrote: > FWIW, I personally prefer to run cmake and ninja //from// the build > directory, i.e. > ``` > $ mkdir ${TOPLEV}/llvm-project/llvm/stage2-prof-gen > $ cd ${TOPLEV}/llvm-project/llvm/stage2-prof-gen > $ CPATH=${TOPLEV}/stage1/install/bin > $ cmake -G Ninja \ > -DLLVM_TARGETS_TO_BUILD=X86 \ > ~~~ > -DCMAKE_INSTALL_PREFIX=${TOPLEV}/stage2-prof-gen/install \ > ../llvm-project/llvm > $ ninja install > ``` > This is the style that was used in `DataFlowSanitizer.rst` below, which you > explicitly moved away from into `-B`/`-S`/`-C` land (where I find it harder > to keep straight all the different extra options that are needed). > > Orthogonally, I'm worried that the advice on lines 73–74 of README.md doesn't > mention `CMAKE_INSTALL_PREFIX`. When I'm building libc++ locally as part of > my development workflow, I //absolutely do not// want to blow away my > computer's default standard library installation; but what //do// I want to > do? Should I use something like `-DCMAKE_INSTALL_PREFIX=$(pwd)/install` as > depicted here? Can I really not get away with "running it out of the > buildroot" the way I'm used to?— I //must// install it somewhere on my system > in order to test it at all? > Re: `-C` `-B` `-S` I really like using those flags because it makes the command relocatable, i.e. it works regardless of where it is being run. As such, it's way more copy-pasteable too. However, since that's orthogonal to this change, I'll revert to the old style just so we have fewer things to argue about. But in general, for libc++/libc++abi/libunwind instructions, you'll find that I want things documented this way for that reason. > Re: `CMAKE_INSTALL_PREFIX` You don't necessarily need to install it in order to test it -- we test libc++ all the time without running the `install-cxx` target. Furthermore, I would expect that most systems nowadays have some sort of integrity protection that prevents you from doing something bad like that. Apple platforms certainly do -- you simply can't overwrite your `libc++.1.dylib` anymore and render your system unusable. Still, I'll add a word of caution to `CMAKE_INSTALL_PREFIX` above even though it's orthogonal to this patch -- it's easy enough to do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119351/new/ https://reviews.llvm.org/D119351 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
