Amir marked an inline comment as not done.
Amir added inline comments.

================
Comment at: clang/CMakeLists.txt:930-937
+                -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
+                -DCMAKE_C_COMPILER=${CLANG_INSTRUMENTED}
+                -DCMAKE_CXX_COMPILER=${CLANGXX_INSTRUMENTED}
+                -DCMAKE_ASM_COMPILER=${CLANG_INSTRUMENTED}
+                -DCMAKE_ASM_COMPILER_ID=Clang
+                -DCMAKE_BUILD_TYPE=Release
+                -DLLVM_ENABLE_PROJECTS=${CLANG_BOLT_INSTRUMENT_PROJECTS}
----------------
MaskRay wrote:
> phosek wrote:
> > I don't think this is sufficient in the general case, we would need to pass 
> > additional variables like `CMAKE_AR` the same way we do for the existing 
> > bootstrap logic, see 
> > https://github.com/llvm/llvm-project/blob/dc549bf0013e11e8fcccba8a8d59c3a4bb052a3b/clang/CMakeLists.txt#L825.
> > 
> > For example, on Fuchsia builders we don't have any system-wide toolchain 
> > installation, instead we manually set  all necessary `CMAKE_<TOOL>` 
> > variables for the first stage, so this call will fail for us because it 
> > won't be able to find tools like the archiver.
> > 
> > Since handling this properly would likely duplicate a lot of the existing 
> > logic from the existing bootstrap logic, I'm wondering if we should instead 
> > try to refactor the existing logic and break it up into macros/functions 
> > which could then be reused here as well.
> Supporting other cmake variables will be awesome. I use something like 
> `-DCMAKE_CXX_ARCHIVE_CREATE="$HOME/llvm/out/stable/bin/llvm-ar qcS --thin 
> <TARGET> <OBJECTS>" -DCMAKE_CXX_ARCHIVE_FINISH=:` to make my build smaller.
Addressed in D133633


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132975

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

Reply via email to