[Openmp-commits] [PATCH] D85742: [libomptarget] Implement host plugin for amdgpu

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

I really have a hard time understanding why all this code is necessary. Browsing it it seems to duplicate a lot of things that are in the OpenMP target runtime or I assumed would be in the underlying driver. Anyway, it is what it is.



================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:1004
+  // begin and end as well as the target name (see the offloading linker script
+  // creation in clang compiler).
+
----------------
There is no linker script anymore, IIRC.


================
Comment at: openmp/libomptarget/src/rtl.cpp:32
+    /* AMDGPU target  */ "libomptarget.rtl.amdgpu.so",
+};
 
----------------
JonChesterfield wrote:
> Discussed in call that these should be in the order in which they were added to the codebase. Thus AMDGPU is at the end as the most recent addition.
> 
> Someone who knows this chronological order should reorder them separate to this patch.
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85742