yaxunl planned changes to this revision. yaxunl added a comment. 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. Then I should add HIP include path in Clang::AddPreprocessingOptions instead of AddClangSystemIncludeArgs. Then I have to add ToolChain::AddPostSystemHIPIncludeArgs since not all ToolChain have RocmInstallation. Basically this will end up as ToolChain having two APIs: one for adding HIP wrapper include args, one for adding HIP include args. 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
