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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
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:
> > > 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.
> > So, for OpenMP 5.0 we should not emit error here.
> > that for OpenMP 5.0 we should emit a warning/error here.
> 
> The last answer contradicts what you said earlier. I expect there is a *not* missing, correct?
> 
> 
> Assuming you do not want an error, which is fine, I still think a warning is appropriate as it seems to me there is never a reason to have a `is_device_ptr` clause for a private value. I mean, it is either a bug by the programmer, e.g., 5 letters of `firstprivate` went missing, or simply nonsensical code for which we warn in other situations as well.
Missed `not`.
These kind of construct are explicitly allowed in OpenMP. And we should obey the standard unconditionally.
Plus, there might be situations where user may require it explicitly. For example, the device pointer is dereferenced in one of the clauses for the subregions but in the deeper subregion it might be used as a private pointer. Why we should emit a warning here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835