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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D120132#3351853 <https://reviews.llvm.org/D120132#3351853>, @tra wrote:

> In D120132#3351391 <https://reviews.llvm.org/D120132#3351391>, @yaxunl wrote:
>
>> 
>
>
>
>> If any input file is HIP program, clang driver will use HIP offload kind for all inputs. This behavior is similar as cuda-clang.
>
> I do not think this is the case as illustrated by the example above. CUDA paths are only added to CUDA compilation. C++ compilation of `b.cc` does not have any of them.

I noticed that with "-c" HIP/CUDA include path is not used for C++ program. However without "-c" HIP/CUDA include path is used for C++ program. Probably this is a bug.

>> Therefore if any input file is HIP program, HIP include path is added for each input file compilation. I think this is acceptable.
>
> I disagree. HIP-specific include path should apply to HIP-specific cc1 compilation only.
>
>> However, when there is only C++ input file, clang driver should not add HIP or CUDA include path.
>
> Agreed. AFAICT this is the case for CUDA.
>
>> The current patch breaks that. That's why I think it needs change.
>
> I agree. Hence my suggestion to add the path via a toolchain-specific `AddClangSystemIncludeArgs`.

In the current patch, AddClangSystemIncludeArgs is modified to add HIP include path. However, there is no good way to know if the current job action is HIP or C++ program.

The signature of AddClangSystemIncludeArgs is

  void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
                                        ArgStringList &CC1Args) const ;

In the case of two input files `a.hip` and `b.cpp`, DriverArgs contain both `a.hip` and `b.cpp`. Clang does not know if the call of AddClangSystemIncludeArgs is for `a.hip` or `b.cpp`. The current patch adds HIP include path for both HIP and C++ programs.

To fix this issue, we either need to add a JobAction argument to AddClangSystemIncludeArgs to let clang know the current call of AddClangSystemIncludeArgs is for HIP program or C++ program, or we need to add HIP include path in a location where the current JobAction is known.


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

https://reviews.llvm.org/D120132