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

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D65835#1623858 <https://reviews.llvm.org/D65835#1623858>, @ABataev wrote:

> In D65835#1623854 <https://reviews.llvm.org/D65835#1623854>, @jdenny wrote:
>
> > In D65835#1623814 <https://reviews.llvm.org/D65835#1623814>, @hfinkel wrote:
> >
> > > In D65835#1623772 <https://reviews.llvm.org/D65835#1623772>, @ABataev wrote:
> > >
> > > > In D65835#1623756 <https://reviews.llvm.org/D65835#1623756>, @kkwli0 wrote:
> > > >
> > > > > In D65835#1622042 <https://reviews.llvm.org/D65835#1622042>, @ABataev wrote:
> > > > >
> > > > > > > I want to be sure we're on the same page: For OpenMP 5.0, should we allow is_device_ptr with the private clauses?
> > > > > >
> > > > > > Yes, since it is allowed by the standard.
> > > > >
> > > > >
> > > > > Umm ... I probably missed some earlier discussions!  What would be the behavior of the following code?
> > > > >
> > > > >   p = omp_target_alloc(...);
> > > > >   #pragma omp target private(p) is_device_ptr(p)
> > > > >     p[...] = ...;   // crash or not?
> > > > >
> > > >
> > > >
> > > > It must crush, I assume. The main problem is that this construct is allowed by the standard.
> > >
> > >
> > > Yep. We should add a warning message for it.
> >
> >
> > Clang currently doesn't permit is_device_ptr with firstprivate either, but I'm not aware of any reason to diagnose that.  Is there?  And then there are privatization clauses like linear and reduction....
>
>
> I assume, we should allow all allowed datasharing clauses along with is_device_ptr clause, including shared, linear, etc.


Agreed.  I was just wondering if anyone saw an obvious issue that might deserve a warning (as in the private plus is_device_ptr case).  I'm not planning to implement warnings right now.  I'm just curious for later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835