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: > > > > 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. ================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8857 + MappableExprsHandler MEHandler(*D, MapperCGF); + MEHandler.generateAllInfoForMapper(BasePointers, Pointers, Sizes, MapTypes); + ---------------- These data can be passed as parameters to the function, no? 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
