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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
