This is an archive of the discontinued LLVM Phabricator instance.

[Clang][OpenMP] Codegen generation for has_device_addr claues.
ClosedPublic

Authored by jyu2 on Sep 20 2022, 2:04 AM.

Details

Summary

This patch add codegen support for the has_device_addr clause. It use the same logic of is_device_ptr. But passing &var instead pointer to var to kernal.

This is follow up of revert of https://reviews.llvm.org/D134186

Diff Detail

Event Timeline

jyu2 created this revision.Sep 20 2022, 2:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2022, 2:04 AM
jyu2 requested review of this revision.Sep 20 2022, 2:04 AM
abhinavgaba added inline comments.Sep 20 2022, 2:19 AM
clang/test/OpenMP/target_has_device_addr_codegen.cpp
352

The kernel argument should be [[K]] instead of [[TMP2]]. And the kernel's definition would need to be updated to take that extra pointer indirection into account.

jyu2 updated this revision to Diff 461650.Sep 20 2022, 12:04 PM

Address @abhinavgaba's comment by setting capture by reference instead by copy.

clang/test/OpenMP/target_has_device_addr_codegen.cpp
352

Thanks @abhinavgaba! Changed

abhinavgaba accepted this revision.Sep 20 2022, 1:13 PM

Thanks, Jennifer. Looks good to me.

This revision is now accepted and ready to land.Sep 20 2022, 1:13 PM

Hi there, I'm trying to fix https://github.com/llvm/llvm-project/issues/59160. The faulty case is basically like the following:

void xoo() {
  short a[10], b[10];
  a[1] = 111;
  b[1] = 111;
#pragma omp target data map(to : a[0 : 2], b[0 : 2]) use_device_addr(a, b)
#pragma omp target has_device_addr(a) has_device_addr(b[0])
  {
    a[1] = 222;
    b[1] = 222;
    // CHECK: 222 222
    printf("%hd %hd %p %p %p\n", a[1], b[1], &a, b, &b);
  }
  // CHECK:111
  printf("%hd %hd %p %p %p\n", a[1], b[1], &a, b, &b); // 111 111 p1d p2d p3d
}

It looks like at runtime, we are trying to copy a (device) pointer to a device pointer by using host to device data transfer. I noticed that's because we have TO flag marked for the argument. However, since a and b are in has_device_addr, we are not supposed to map the two variables right?

clang/lib/CodeGen/CGOpenMPRuntime.cpp
9098

The variable used in has_device_addr indicates it already has device address and can be accessed directly. Why do we need OMP_MAP_TO here? OMP_MAP_TO indicates we are gonna transfer data to the device right?

clang/lib/CodeGen/CGOpenMPRuntime.cpp
9098

IIUC, https://reviews.llvm.org/D141627 would fix it. Could you please take a look if it makes sense?