[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D65835#1639617 <https://reviews.llvm.org/D65835#1639617>, @jdenny wrote:

> In D65835#1639612 <https://reviews.llvm.org/D65835#1639612>, @ABataev wrote:
>
> > In D65835#1639593 <https://reviews.llvm.org/D65835#1639593>, @jdenny wrote:
> >
> > > In D65835#1639585 <https://reviews.llvm.org/D65835#1639585>, @ABataev wrote:
> > >
> > > > In D65835#1639584 <https://reviews.llvm.org/D65835#1639584>, @jdenny wrote:
> > > >
> > > > > I want to be sure we're on the same page.  Due to the changes I just backed out, the following two examples now generate different code:
> > > > >
> > > > >   int a = 0;
> > > > >   #pragma omp target map(a)
> > > > >   #pragma omp teams firstprivate(a)
> > > > >     ;
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >   int a = 0;
> > > > >   #pragma omp target teams firstprivate(a) map(a)
> > > > >     ;
> > > > >
> > > > >
> > > > > The difference is whether `a` is passed by reference (the first case) or value (the second case) to the offloading function.
> > > > >
> > > > > Is that fine for you?
> > > >
> > > >
> > > > No, this is what I warned about. We shall have the same codegen just like in the first case, the value must be passed by reference and mapped as tofrom.
> > >
> > >
> > > If I add back those changes I just backed out, we get the same codegen.  Is that what you want?
> >
> >
> > Those 2 cases must result in the same codegen. But I rather doubt we need your previous changes. Check `Sema::isOpenMPCapturedByRef` instead, required functionality must be handled in this function.
>
>
> That's the focus of my previous changes.  The rest just supports the changes there.


We don't need this new level counter to correctly handle this situation. Just check for the combined target directive in `Sema::isOpenMPCapturedByRef` and return true if it is mapped as to from or just from. The change must be very simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835