[Openmp-commits] [PATCH] D70971: [libomptarget] Build a minimal deviceRTL for amdgcn

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added inline comments.


================
Comment at: openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt:16-18
+  $ENV{HOME}/rocm/aomp
+  /opt/rocm/aomp
+  /usr/lib/rocm/aomp
----------------
JonChesterfield wrote:
> ABataev wrote:
> > We can't build the runtime with regular clang?
> Sure, it builds fine with trunk. I'm using a local build of clang 9.0 on one machine and the openmp/fortran fork on a second.
> 
> The find package seems to be a list of anywhere that clang might plausibly be found. Copied straight out of our fork, happy to remove any entries you don't like the look of.
> 
> The NVPTX_CUDA_COMPILER_DIR doesn't seem especially appropriately named. I use the same compiler for nvptx and for amdgcn.
Can we then just use the clang (I think, you proposed this for nvcc) to build the runtime? Why shall we lookup for `rocm/aomp`?


================
Comment at: openmp/libomptarget/deviceRTLs/amdgcn/src/device_environment.h:18-26
+struct omptarget_device_environmentTy {
+  int32_t debug_level;   // gets value of envvar LIBOMPTARGET_DEVICE_RTL_DEBUG
+                         // only useful for Debug build of deviceRTLs 
+  int32_t num_devices;   // gets number of active offload devices 
+  int32_t device_num;    // gets a value 0 to num_devices-1
+};
+
----------------
JonChesterfield wrote:
> ABataev wrote:
> > Agan this thing. Can we make it common if it is really required? Again, why do we need these new fields? Could you remove it from the current patch?
> It's different on nvptx and amdgcn, so not a great candidate for common. The extra fields are used to implement a couple of functions that are stubs in trunk which you weren't sure are necessary for nvptx.
Maybe, it is better to exclude this from this patch? Maybe it would be good to have these fields for nvptx too or we don't need them to for amdgcn. Could you remind where we discussed this structure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70971