[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+      llvm::sys::path::append(P, "llvm-libc");
+      CmdArgs.push_back("-c-isystem");
+      CmdArgs.push_back(Args.MakeArgString(P));
----------------
Ensuring the right include order will be tricky. Interaction between the headers provided by llvm-libc vs the system headers will be interesting if we end up mixing the headers. It may be less problematic compared to the C++ standard library, but I doubt that mixing two implementations would work well here, either. 

So, the major question I have -- does adding include path here guarantees that we're not picking up the host headers? I do not see any changes that would exclude system headers from include paths.
If we do have include paths leading to both llvm-libc and the host headers, what's expected to happen if user code ends up including a header that's not present in  llvm-libc? Is that possible?



================
Comment at: clang/test/Driver/gpu-libc-headers.c:14
+// RUN:     FileCheck %s --check-prefix=CHECK-HEADERS
+// CHECK-HEADERS: "-cc1"{{.*}}"-c-isystem" "{{.*}}include/llvm-libc"
+
----------------
I think here we want to test for not just the presence of `include/llvm-libc`, but also that it's in the correct position relative to other include paths. 

We may want something similar to what we have in clang/test/Driver/hip-include-path.hip


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973