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