[libc-commits] [PATCH] D152592: [libc] Place headers in the right include directory

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra added a subscriber: jhuber6.
sivachandra added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/CMakeLists.txt:30
+  endif()
   set(LIBC_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
 else()
----------------
phosek wrote:
> @sivachandra When `LIBC_TARGET_ARCHITECTURE_IS_GPU`, would it make sense to use `gpu-none-llvm` instead of `LLVM_DEFAULT_TARGET_TRIPLE`? It seems like we're effectively treating `gpu-none-llvm` as a target triple but I wanted to confirm first before making the change.
If building for the GPU, I think we want the target triple to be fixed to `gpu-none-llvm` and `LLVM_DEFAULT_TARGET_TRIPLE` does not matter. I will let @jhuber6 to confirm.


================
Comment at: libc/CMakeLists.txt:34
+    set(LIBC_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
     set(LIBC_LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
   elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
----------------
phosek wrote:
> @sivachandra Would it make sense to use `${CMAKE_CURRENT_BINARY_DIR}/lib` in this case for consistency?
Agreed.


================
Comment at: libc/include/CMakeLists.txt:532
+add_dependencies(libc-headers ${all_install_header_targets} copy-libc-headers)
+target_include_directories(libc-headers SYSTEM INTERFACE ${LLVM_BINARY_DIR}/include)
----------------
What is this `target_include_directories` doing? I can see why it could be `${LIBC_INCLUDE_DIR}` but I do not understand why `${LLVM_BINARY_DIR}/include`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152592