[Openmp-commits] [PATCH] D51107: [LIBOMPTARGET] Add support for mapping of lambda captures.

[Alexandre Eichenberger via Phabricator via Openmp-commits]

AlexEichenberger added a comment.

In https://reviews.llvm.org/D51107#1211035, @Hahnfeld wrote:

>




> I agree that this doesn't work with the current code. IIRC the reason is that libomptarget has two loops dealing with `map` clauses: one in `target_data_begin` (which also handles `MEMBER_OF`) and the one in `target` which this patch is modifying. The problem is that `firstprivate` arguments are handled in the second, so the one in `target_data_begin` (which runs first) doesn't have a device address to resolve entries with `MEMBER_OF`. IMO the proper solution would be to have all allocations in one single loop in `target_data_begin`. The loop in `target` should only deal with constructing the arguments passed to the device.
> 
> To implement this efficiently we need a temporary data structure for each mapping so that we can store the device address allocated in `target_data_begin`. That pointer could then be queried in `target` to populate `tgt_args`.
>  (Ideally the data structure would wrap each entry in the `args_base`, `args`, `arg_sizes`, and `arg_types` arrays into a nice object. This would have functions to hide all this bitmasking to find out the type.)
> 
> Long story short, I think that lambdas are indeed `firstprivate`. What needs to be fixed are `firstprivate` "mappings" together with `MEMBER_OF` (@AlexEichenberger  this already works in lomp, right?).

While I am not 100% familiar with the libomptarget code yet, it is possible to handle the beginning of a target construct like a target begin (phase 1) , and the end of a target construct like a target end (phase 3), with the invocation of the target being phase 2. Keeping a data structure that hold the data from phase 1 to phase 3 is quite advantageous, as locating maps is generally expensive. Flow of info from phase 1 to phase 2 is minimal (only the list of parameters, which are indicated by a flag in the arg_types parameter.

Incidentally, having a data structure becomes necessary once async is implemented, as there are then a lot more state to remember (e.g. events associated with the termination of specific data transfers, book keeping for deleting temporary buffers once data is copied over...)

In my mind, MEMBER_OF does not imply MAP, it can be used for firstprivate just the same (though member_of firstprivate cannot be given by a user, as only map can have alloc of a whole structure, with to, from, tofrom for subsets of the structure).

PTR_AND_OBJ does not imply a deep copy, if the OBJ is already mapped, it will find it, increment the ref count on entry/decrement on exit. If we need a PTR_AND_MAPPED_OBJ, where NULL is set if the object is not present, that is something we could do. Because of the oncoming unified mem, PTR_AND_MAPPED_OBJ would simply preserve the original address (no attachment) with the requires unified_shared_memory.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D51107