[Openmp-commits] [PATCH] D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs.

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D14254#949985, @guansong wrote:

> Here is a general comment, we should follow Alex's suggestion to make a generic GPU device and extend that to NVPTX and AMDGCN. My current idea is:
>
> 1. think all the current spell of nvptx as generic_gpu
> 2. add template to some functions, and instantiate them with nvptx and amdgcn required types
> 3. for example the warp size in nvptx is 32 while in amdgcn is 64
>
>   Thoughts?


No, not in this patch which already is very large:

1. The size makes it hard to review and update. This slows down the entire process of getting this into trunk.
2. This patch is already circumventing the "normal" LLVM process of well-tested patches that introduce one functionality after another. (That said, I still think we don't have an alternative to the current approach: The whole interface is set and already partially implemented in the compiler, so we need to do this in one pass because splitting makes no sense.)

I'll happily review patches that introduce support for AMDGCN after a first version of the code is committed but I refuse to let this in with this revision.


Repository:
  rL LLVM

https://reviews.llvm.org/D14254