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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
