[PATCH] D148254: [OpenMP][Flang][Semantics] Add semantics support for USE_DEVICE_PTR clause on OMP TARGET DATA directive.

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan requested changes to this revision.
kiranchandramohan added a comment.
This revision now requires changes to proceed.

Thanks for the patch. I have a few comments inline.



================
Comment at: flang/lib/Semantics/resolve-directives.cpp:440
+  bool Pre(const parser::OmpClause::UseDevicePtr &x) {
+    ResolveOmpObjectList(x.v, Symbol::Flag::OmpUseDevicePtr);
+    return false;
----------------
We probably need a debug-dump-symbols test to show that the SymbolFlag is applied.


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:510
 
   static constexpr Symbol::Flags ompFlagsRequireNewSymbol{
       Symbol::Flag::OmpPrivate, Symbol::Flag::OmpLinear,
----------------
>From the standard:
> Pointers that appear in a `use_device_ptr` clause are privatized and the device pointers to the corresponding list items in the device data environment are assigned into the private versions.

Since the pointer in the `use_device_ptr` is privatised, it is best to follow the representation for privatised variables i.e represent them with a new Symbol. It is probably good to add `Symbol::Flag::OmpUseDevicePtr` to `ompFlagsRequireNewSymbol`.


================
Comment at: flang/test/Semantics/OpenMP/use_device_ptr.f90:15
+   !$omp target data map(tofrom: a, arrayB) use_device_ptr(b)
+      call c_f_pointer(b, arrayB)
+      a = arrayB
----------------
If the location pointed to by `b` inside the region is different from the location pointed outside to by `b` then it is probably better to create a new symbol for `b` inside the region.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148254