ABataev 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, ---------------- 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? 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