yaxunl added a comment. In D120132#3349538 <https://reviews.llvm.org/D120132#3349538>, @tra wrote:
> In D120132#3345534 <https://reviews.llvm.org/D120132#3345534>, @yaxunl wrote: > >> I just found one issue with the current patch. It adds HIP include path for >> non-HIP programs. >> >> We should only add HIP include path for JobAction with HIP offloading kind. >> However, AddClangSystemIncludeArgs is not per job action. >> I feel I should not complicate AddClangSystemIncludeArgs API by making it >> accept a JobAction argument. > > I'm not sure I understand. In general we do want host and device-side > compilations to be as close as we can make them and that includes include > paths. `AddClangSystemIncludeArgs` is a toolchain method and does exactly > what we need -- add the same include path to both host and device > compilations when HIP (or CUDA) toolchain is used. > I don't quite see how you could end up with a HIP toolchain in a non-HIP > compilation. > > What do I miss? Users may use clang driver to compile HIP program and C++ program with one clang driver invocation, e.g. clang --offload-arch=gfx906 a.hip b.cpp Clang driver will create job actions for a.hip and b.cpp separately. The job actions for a.hip have HIP offload kind. The job actions for b.cpp have 'none' offload kind. Only job actions having HIP offload kind should have HIP include paths, otherwise, even if clang driver is used for compiling one single C++ program, it will add HIP include path. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120132/new/ https://reviews.llvm.org/D120132 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
