[PATCH] D120132: [HIP] Fix HIP include path

[Yaxun Liu via Phabricator via cfe-commits]

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