tra added a comment.

In, @yaxunl wrote:

> Yes we already have a basic working implementation of HIP compiler due to 
> Greg's work.

That is great, but it's not necessarily true that all these changes will make 
it into clang/llvm as is. LLVM/Clang is a community effort and it helps a lot 
to get the changes in when the community understands what is it you're planning 
to do. I personally am very glad to see AMD moving towards making clang a 
viable compiler for AMD GPUs, but there's only so much I'll be able to do to 
help you with reviews if all I have is either piecemeal patches with little 
idea how they all fit together or one humongous patch I would have no time to 
dive in and really understand. Considering that compilation for GPU is a fairly 
niche market my bet is that your progress will be bottlenecked by the code 
reviews. Whatever you can do to make reviewers jobs easier by giving more 
context will help a lot with upstreaming the patches.

> I will either update 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.

Fair enough. I'll wait for the rest of the patches. If you have multiple 
pending patches, it helps if you could arrange them as dependent patches in 
phabricator. It makes it easier to see the big picture.

> 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?

I'm not sure I understand. Could you elaborate?

cfe-commits mailing list

Reply via email to