[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

Tue Dec 17 07:32:19 PST 2019

ABataev added inline comments.

================
Comment at: clang/include/clang/AST/OpenMPClause.h:4918
                         const OMPMappableExprListSizeTy &Sizes)
-      : OMPMappableExprListClause(OMPC_map, Locs, Sizes, &MapperQualifierLoc,
-                                  &MapperIdInfo),
+      : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+                                  &MapperQualifierLoc, &MapperIdInfo),
----------------
lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > Do we really need to set `HasMapper` to `true` unconditionally here and in other places?
> > > For `map`, `to`, and `from` clauses, they can have mappers, so it's set to `true`. For `is_device_ptr` and `use_device_ptr`, it's set to `false`.
> > So, it completely depends on the clause kind, right? If so, can we just remove it and rely on the clause kind?
> This is used in `OMPMappableExprListClause` which is inherited by all these clauses. Do you mean to check the template type there to get the clause kind?
I see. I think it is better to rename it to `ClauseSupportsMapper` (or something like this) and make it `const`. Currently, it is confusing. I thought that this flag shows the use of the mapper in the clause.

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

https://reviews.llvm.org/D67833