[PATCH] D154123: [HIP] Start document HIP support by clang

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 5 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/docs/HIPSupport.rst:24
+
+Clang supports HIP on `ROCm platform <https://rocm.docs.amd.com/en/latest/#>`_.
+
----------------
keryell wrote:
> keryell wrote:
> > I thought the idea of HIP was to also target Nvidia GPU? Otherwise I do not understand "portable applications for GPUs" above.
> Thinking to the fact this is used by other projects like https://github.com/CHIP-SPV/chipStar this is even wider but at the same time that project is not up-streamed.
will add a brief description of HIP support on other devices.


================
Comment at: clang/docs/HIPSupport.rst:33
+
+   clang++ -c --offload-arch=gfx906 -xhip test.cpp -o test.o
+
----------------
keryell wrote:
> I suggest giving also a simpler example without `-c` to generate directly an executable without split compile and link.
> Also avoid using `test` as an example as it is confusing as it is also a shell builtin on Unix and it might take sometime to understand why the `test` does not use the GPU. :-)
will do


================
Comment at: clang/docs/HIPSupport.rst:35-37
+This command will compile a .cpp file with the -xhip option to indicate that
+the source is a HIP program. However, if the file has a .hip extension, you
+don't need the -xhip option. Clang will automatically recognize it as a HIP
----------------
keryell wrote:
> 
will fix


================
Comment at: clang/docs/HIPSupport.rst:141
+     - Defined with a value of 1 when the target device lacks support for HIP image functions.
+   * - ``HIP_API_PER_THREAD_DEFAULT_STREAM``
+     - Defined when the GPU default stream is set to per-thread mode.
----------------
keryell wrote:
> Should this one be with underscores too?
> 
This is due to design inconsistency. The same with `__HIP_NO_IMAGE_SUPPORT`.

Since these two macros are new and not widely used, renaming them won't cause significant interruptions. I will let clang emit `__HIP_NO_IMAGE_SUPPORT__` and `__HIP_API_PER_THREAD_DEFAULT_STREAM__` and make the current ones alias and document them as deprecated.

The macros for HIP memory scope were designed by following the naming convention of clang atomic macros e.g. `__ATOMIC_RELAXED`, `__OPENCL_MEMORY_SCOPE_WORK_ITEM`, therefore they will not be changed.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154123/new/

https://reviews.llvm.org/D154123