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

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added a comment.

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

> In https://reviews.llvm.org/D51107#1210813, @ABataev wrote:
>
> > I tried it, it won't work.
> >
> > 1. Lambdas are captured as firstprivates, thus we cannot translate the host address of the lambda to the device address. `MEMBER_OF` does not work here because of that.
>
>
> 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?).


BTW, the main problem here is that there is no mapping between host pointer and device pointer for the firstprivates, just like for the mapped data. We just need to rework the way we allocate the memory for the firstprivate to make `MEMBER_OF` to work.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D51107