lildmh added inline comments.
================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8739 +/// // For each component specified by this mapper: +/// if (currentComponent.hasMapper()) +/// (*currentComponent.Mapper())(rt_mapper_handle, arg_base, arg_begin, ---------------- ABataev wrote: > lildmh wrote: > > ABataev wrote: > > > Currently `currentComponent` is generated by the compiler. But can we > > > instead pass this data as an extra parameter to this `omp_mapper` > > > function. > > Emm, I think this scheme will be very difficult and inefficient. If we pass > > components as an argument of `omp_mapper` function, it means that the > > runtime needs to generate all components related to a map clause. I don't > > think the runtime is able to do that efficiently. On the other hand, in the > > current scheme, these components are naturally generated by the compiler, > > and the runtime only needs to know the base pointer, pointer, type, size. > > etc. > With the current scheme, we may end with the code blowout. We need to > generate very similar code for different types and variables. The worst thing > here is that we will be unable to optimize this huge amount of code because > the codegen relies on the runtime functions and the code cannot be inlined. > That's why I would like to move as much as possible code to the runtime > rather than to emit it in the compiler. I understand your concerns. I think this is the best we can do right now. The most worrisome case will be when we have nested mappers within each other. In this case, a mapper function will call another mapper function. We can inline the inner mapper functions in this scenario, so that these mapper function can be properly optimized. As a result, I think the performance should be fine. ================ Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8866-8867 + createRuntimeFunction(OMPRTL__tgt_mapper_num_components), OffloadingArgs); + llvm::Value *ShiftedPreviousSize = + MapperCGF.Builder.CreateShl(PreviousSize, MapperCGF.Builder.getInt64(48)); + ---------------- ABataev wrote: > I don't like this code very much! It hides the logiс ща the MEMBER_OF flag > deep inside and it is going to be very hard to update it in future if there > are some changes in the flags. Add a function to calculate this offset. Also modify another existing place using the hard coded number 48. 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
