jhuber6 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));
----------------
tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > 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?
> > > 
> > Right now I'm just kind of relying on an expectation that since this will 
> > be the first `c-isystem` path set, then it will pull in these libraries 
> > first if they exist. It's not captured by the tests, but compiling with 
> > `-v` shows this path being used first in my experience. So, theoretically, 
> > if there is an implementation of said header in this location, it will be 
> > picked up before anything else. Otherwise it'll just search the other 
> > standard locations.
> I think this will be a problem. We're cross-compiling here and for that to 
> work reliably we need to make sure that only target headers are in effect. 
> The problem is that we likely just do not have sufficiently complete set of 
> headers for the GPU. Do we? I have no idea what exactly llvm-libc provides 
> and whether it is sufficient for normal user code to cross-compile for a GPU. 
> 
> It would be interesting to try to compile some C++ code which would include 
> commonly used, but generally target-agnostic, headers like <vector> <complex> 
> <algorithm>, etc and check whether we end up pulling in any system headers. 
> Then check what happens if we do not have system headers available at all.
No, it's definitely not complete. Some headers work on the GPU, most break in 
some way or another. The only ones `llvm-libc` provides currently is `string.h` 
and `ctype.h`. But, I figured this wouldn't be a problem since it would just go 
to the system headers anyway if we didn't provide them. So we are merely 
replacing maybe broken with probably works.

I was talking with Johannes and he brings up other issues about the idea of 
host-device compatibility between these headers. Since, fundamentally, right 
now `libc` generates its own headers and needs to generate its own headers to 
function. But there can be a problem when migrating data between the host and 
the device is the headers on the host differ somewhat to those on the device. 
I'm not sure what a good overall solution to that problem is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

Reply via email to