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

tra added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1196-1197 + // If we are compiling for a GPU target we want to override the system headers + // with ones created by the 'libc' project if present. + if (!Args.hasArg(options::OPT_nostdinc) &&

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf263bd8f7d4c: [Clang] Implicitly include LLVM libc headers for the GPU (authored by jhuber6). Changed prior to commit:

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

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1196-1197 + // If we are compiling for a GPU target we want to override the system headers + // with ones created by the 'libc' project if present. + if (!Args.hasArg(options::OPT_nostdinc) &&

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

tra accepted this revision. tra added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1196-1197 + // If we are compiling for a GPU target we want to override the system headers + // with ones created by the

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

jhuber6 added a comment. ping. 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

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

jhuber6 updated this revision to Diff 509090. jhuber6 added a comment. Changing to use the `gpu-none-llvm` subfolder name that @sivachandra recommended. Also adding a `--sysroot` argument to show that this include path shows up first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

jhuber6 added a comment. In D146973#4228070 , @tra wrote: > I'm OK with injecting the path *now* with an understanding that it's a > short-term "happens to work" way to move forward while we're working on a > better solution. So, the proposed path

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

tra added a comment. I'm OK with injecting the path *now* with an understanding that it's a short-term "happens to work" way to move forward while we're working on a better solution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146973/new/

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

aaron.ballman added a comment. In D146973#4227470 , @jhuber6 wrote: > In D146973#4227433 , @aaron.ballman > wrote: > >> I am not asking you to implement a library based off another >> implementation's

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

jhuber6 added a comment. In D146973#4227433 , @aaron.ballman wrote: > I am not asking you to implement a library based off another implementation's > specification. I am relaying implementation experience with the design you've > chosen for your

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

aaron.ballman added a comment. In D146973#4227266 , @jhuber6 wrote: > In D146973#4227114 , @aaron.ballman > wrote: > >> Hmmm, I've had experience with SYCL as to how it goes when you have >> difference between

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

jhuber6 added a comment. In D146973#4227114 , @aaron.ballman wrote: > Hmmm, I've had experience with SYCL as to how it goes when you have > difference between host and device; those kinds of bugs are incredibly hard > to track down. Pointer sizes

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

aaron.ballman added a comment. In D146973#4225645 , @jhuber6 wrote: > In D146973#4225641 , @aaron.ballman > wrote: > >>> This lets offloading languages such as OpenMP use the system string.h when >>> compiling

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

jdoerfert added a comment. @sivachandra Host and device should agree on all ABI related properties of types or things turn badly, fast. That has to be a given for things to work, and from there, users expect types to be copyable, and usable on both sides. @jhuber6 There are things we might

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

jhuber6 added a comment. In D146973#4225983 , @jdoerfert wrote: > I said this before, many times: > > We don't want to have different host and device libraries that are > incompatible. > Effectively, what we really want, is the host environment to just

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

sivachandra added a comment. In D146973#4225983 , @jdoerfert wrote: > For most of libc, we might get away with custom GPU headers but eventually it > will break "expected to work" user code, at the latest when we arrive at > libc++. > A user can, right

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

jdoerfert added a comment. I said this before, many times: We don't want to have different host and device libraries that are incompatible. Effectively, what we really want, is the host environment just work on the GPU, that includes extensions in the host headers, macros, taking the address of

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

jhuber6 added a comment. In D146973#4225641 , @aaron.ballman wrote: >> This lets offloading languages such as OpenMP use the system string.h when >> compiling for the host and then the LLVM libc string.h when targeting the >> GPU. > > How do we avoid

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

aaron.ballman added a comment. > This lets offloading languages such as OpenMP use the system string.h when > compiling for the host and then the LLVM libc string.h when targeting the GPU. How do we avoid ABI issues when the two headers get sufficiently out of sync? (In general, I'm pretty

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

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)); jhuber6 wrote: > sivachandra wrote: > > tra

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

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)); sivachandra wrote: > tra wrote: > >

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

jhuber6 added a comment. In D146973#4225300 , @tschuett wrote: > Could you hide the amdgpu and nvptx somewhere libc here `clang > -print-resource-dir` in two different directories? One for AMD, one for > NVPTX. So, right now this header is installed

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

tschuett added a comment. Could you hide the amdgpu and nvptx somewhere here `clang -print-resource-dir` in two different directories? One for AMD, one for NVPTX. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146973/new/

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

sivachandra 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: > sivachandra wrote: > >

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

JonChesterfield added a comment. Can we default to freestanding on, or just document that freestanding is a good idea, instead of hacking with include behaviour directly? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146973/new/

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

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)); sivachandra wrote: > jhuber6 wrote: > > tra

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

sivachandra 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)); jhuber6 wrote: > tra wrote: > >

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

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

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

tra added a subscriber: echristo. 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)); jhuber6

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

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: > Ensuring the right include

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

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

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

jhuber6 added a comment. I'm not sure if there's a better way to provide these headers. Like if we let the `libc` project output to the Clang resource directory or some other neatly nested directory. Right now this just picks up `bin/clang/../include/llvm-libc`. Repository: rG LLVM Github

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

jhuber6 created this revision. jhuber6 added reviewers: tra, yaxunl, JonChesterfield, sivachandra, MaskRay, jdoerfert, tianshilei1992. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, jplehr, sstefan1. Herald added a project: clang.