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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to