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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
