[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9566
+    MEHandler.generateAllInfo(BasePointers, Pointers, Sizes, MapTypes,
+                              CapturedVarSet, /*PresentModifierOnly=*/true);
 
----------------
jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > jdenny wrote:
> > > > > In today's OpenMP/LLVM call, it was pointed out that this issue represents a general Clang bug for map clauses: map clauses generally shouldn't be ignored just because their variables aren't referenced in the construct.
> > > > > 
> > > > > Here, I tried to handle this issue only in the case of a `present` modifier because I previously didn't consider whether the existing behavior for other map types was a bug.  My solution here was similar to what's done for `omp target data`, which doesn't have captures at all.  Perhaps, this should generalized to other map types.
> > > > > 
> > > > > Another possibility might be to expand what's captured in Sema.
> > > > It must be fixed in a separate patch. We definitely should not modify sema here, just the codegen. Currently, it iterates through all captures in region, also need to iterate through the explicit maps.
> > > Thanks for the response.  I'd be happy to generalize and extract into a new patch.
> > > 
> > > It looks like all I need to extract is the changes to `generateAllInfo` and `emitTargetCall` except I wouldn't have the `PresentModifierOnly` restriction.  Of course, I would need to add tests.  Does all that sound about right?  If so, I'll work on it.
> > Yes, something like this. Most probably, you won't need to add new tests, just modify the existing ones.
> OK, I'm working on this.  One point from the call I'm not confident about:  Is this change just for globals or locals too?  That is, which variables in the following examples should have map entries?
> 
> ```
> int x;
> void fn() {
>   int y;
>   #pragma omp target map(x,y)
>   {
>     // no references to x and y are lexically contained
>   }
> }
> ```
I think, both. We need to have references in the data environment for all explicitly mapped variables.


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

https://reviews.llvm.org/D83061