yaxunl added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:531-532 + DriverArgs.hasArg(options::OPT_nostdlibinc)) { + CC1Args.push_back("-internal-isystem"); + CC1Args.push_back(HipIncludePath); + } ---------------- tra wrote: > My impression, after reading the problem description, is that the actual > issue is that we're using `-internal-isystem` for the HIP SDK includes , not > that we add the include path to them. > > Instead of trying to guess whether we happen to match some hardcoded path > where we think the standard headers may live, I'd rather use `-I` or its > internal equivalent, if we have it. Hardcoded paths *will* be wrong for > someone. E.g. I'm pretty sure `/usr/anything` is not going to work on > windows. Of for our internal builds. changing `-internal-isystem` to `-I` is a solution, as the same path showing up first with both `-I` then with `-internal-isystem` will be treated as `-internal-isystem` and placed in the latter position. However, one drawback is that this may cause regression due to warnings emitted for HIP headers. I think I may let AddHIPIncludeArgs return the HIP include path instead of adding it right away, then let clang add it after the system include paths. I may rename AddHIPIncludeArgs as AddHIPWrapperIncludeArgsAndGetHIPIncludePath. What do you think? Thanks. 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