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

Wed Jul 15 09:08:57 PDT 2020

jdenny added inline comments.

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9566
+    MEHandler.generateAllInfo(BasePointers, Pointers, Sizes, MapTypes,
+                              CapturedVarSet, /*PresentModifierOnly=*/true);

----------------
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.

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

https://reviews.llvm.org/D83061