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

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

jdenny added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835