yaxunl added a comment.

In https://reviews.llvm.org/D44984#1050526, @tra wrote:

> The changes appear to cover only some of the functionality needed to enable 
> HIP support. Do you have more patches in queue? Having complete picture would 
> help to make sense of the overall plan.
>  I did ask for it in https://reviews.llvm.org/D42800, but I don't think I've 
> got the answers. It would help a lot if you or @gregrodgers could write a doc 
> somewhere outlining overall plan for HIP support in clang, what are the main 
> issues that need to be dealt with, and at least a general idea on how to 
> handle them.
> As far as "add -x hip, and tweak runtime glue codegen" goes, the change looks 
> OK, but it's not very useful all by itself. It leaves a lot of other issues 
> unsolved and no clear plan on whether/when/how you are planning to deal with 
> them.
> As things stand right now, with this patch clang will still attempt to 
> include CUDA headers, which, among other things will provide 
> threadIdx/blockIdx and other CUDA-specific features.
>  Perhaps it would make sense to disable pre-inclusion of CUDA headers and, 
> probably, disable use of CUDA's libdevice bitcode library if we're compiling 
> with -x hip (i.e. -nocudainc -nocudalib).
>  If you do depend on CUDA headers, then, I suspect, you may need to adjust 
> some wrapper headers we use for CUDA and that change should probably come 
> before this one.

Hi Artem, I am responsible for upstreaming Greg's work and addressing 
reviewers' comments.

Yes we already have a basic working implementation of HIP compiler due to 
Greg's work. I will either update https://reviews.llvm.org/D42800 or create a 
new review about the toolchain changes for compiling and linking HIP programs. 
Essentially HIP has its own header files and device libraries which are taken 
care of by the toolchain patch.

Since the header file and library seem not to affect this patch, is it OK to 
defer their changes to be part of the toolchain patch?



cfe-commits mailing list

Reply via email to