[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

Lingda Li via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 10 10:36:36 PDT 2019


lildmh marked an inline comment as done.
lildmh added inline comments.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper.<type_name>.<mapper_id>.(void *rt_mapper_handle,
+///                                           void *base, void *begin,
----------------
ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > lildmh wrote:
> > > > > > ABataev wrote:
> > > > > > > This function looks like the universal one, regardless of the type `<type_name>` specifics. Do we really need to generate it for each particular type and mapper? Or we could use the same function for all types/mappers?
> > > > > > I think we need a particular mapper function for each type and mapper, because the code generated within the mapper function depends on what type and what mapper it is.
> > > > > Hmm, maybe I'm wrong but I don't see significant mapper or type-specific dependencies in this mapper function. It uses the pointer to type and size of the type, but this information can be generalized, I think. Could you point the lines of code that are type and mapper specific?
> > > > Code between line 8857-8965 is type and mapper specific. For instance, `generateAllInforForMapper` depends on the map clauses associated with the mapper and the internal structure of struct/class type, and generates difference code as a result. `BasePointers.size()` also depends on the above things.
> > > Most of these data can be passed as parameters to the function. It would be good, if we could move this function to the libomptaret library and reduce the number of changes (and, thus, complexity) of the compiler itself. It is always easier to review and to maintain the source code written in C/C++ rather than the changes in the compiler codegen. Plus, it may reduce the size of the final code significantly, I assume. I would appreciate it if you would try to move this function to libomptarget.
> > Hi Alexey, I don't think libomptarget can do this efficiently. For example,
> > 
> > ```
> > class C {
> >   int a;
> >   double *b
> > }
> > 
> > #pragma omp declare mapper(C c) map(c.a, c.b[0:1])
> > ```
> > 
> > The codegen can directly know there are 2 components (c.a, c.b[0]) in this mapper function (3 actually when we count the pointer), and it can also know the size, starting address, map type, etc. about these components. Passing all these information to libomptarget seems to be a bad idea. Or did I get your idea wrong?
> > 
> Yes, I understand this. But can we pass some additional parameters to this function so we don't need to generate a unique copy of almost the same function for all types/mappers? 
For different types/mappers, the skeleton of mapper functions are similar (i.e., the things outlined in the comment here). I would say most other code is unique, for instance, the code to prepare parameters of call to `__tgt_push_mapper_component`. These code should be much more compared with the skeleton shown here. I cannot think of a way to reduce the code by passing more parameters to this function. Please let me know if you have some suggestions.


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

https://reviews.llvm.org/D59474





More information about the cfe-commits mailing list