zhuhan0 added a comment.

In D81176#2106382 <https://reviews.llvm.org/D81176#2106382>, @yaxunl wrote:

> In D81176#2105944 <https://reviews.llvm.org/D81176#2105944>, @zhuhan0 wrote:
>
> > This broke a test `clang/test/Tooling/clang-check-offload.cpp` for a 
> > critical Linux distro at Facebook. With this change, the test adds a 
> > `-include __clang_hip_runtime_wrapper` argument. The wrapper includes some 
> > standard c++ headers, but our distro don't have those headers in the 
> > default include paths, thus causing a break.
> >
> > I notice this behavior doesn't happen for CUDA tests, which also rely on a 
> > similar `__clang_cuda_runtime_wrapper`. I think what's causing the 
> > difference is the different handling of `nogpuinc/nogpulib` option. My 
> > knowledge on this area is limited, so correct me if I'm wrong. CUDA seems 
> > to respect `nogpuinc` and doesn't include its wrapper if the flag is 
> > provided: 
> > https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/Cuda.cpp#L255.
> >  But based on this change, HIP does things differently: 
> > https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/AMDGPU.cpp#L226.
> >
> > If I modify `RocmInstallationDetector::AddHIPIncludeArgs` to also respect 
> > `nogpuinc/nogpulib`, the test will pass for us. Is it a mistake for HIP to 
> > always include the wrapper file? Could you provide a fix for this issue? 
> > Thanks!
>
>
> Thanks for investigating the issue. It makes sense to respect nogpuinc and 
> nogpulib. fixed by 2580635bd2f3c0527353e4d7823326cd9f92ff7c 
> <https://reviews.llvm.org/rG2580635bd2f3c0527353e4d7823326cd9f92ff7c>


It works! Thanks for the quick fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81176



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to