[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

> Maybe, but I haven't found any statement in either version that states that map restrictions apply to is_device_ptr.

`is_device_ptr` is a kind of mapping clause. I assume we can extend the restrictions for `map` clause to this clause too.

> Another question is whether the restriction would make sense. For example, does it ever make sense to specify both is_device_ptr and firstprivate for the same variable on a target construct?

On a `target` construct - no. On a `target parallel` - yes. This may be important especially in unified shared memory mode.



================
Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
       ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // expected-error{{private variable cannot be in a is_device_ptr clause in '#pragma omp target parallel for' directive}} expected-note{{defined as private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
     for (int ii=0; ii<10; ii++)
----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > I think this should cause an error or at least a warning. Telling the compiler `ps` is a device pointer only to create a local uninitialized shadowing variable seems like an error to me.
> > It is allowed according to OpenMP 5.0. Private copy must be created in the context of the parallel region, not the target region. So, for OpenMP 5.0 we should not emit error here.
> What does that mean and how does that affect my reasoning?
It means, that for OpenMP 5.0 we should emit a warning/error here. It is allowed according to the standard, we should allow it too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835