[Openmp-commits] [PATCH] D68100: [OpenMP 5.0] declare mapper runtime implementation

[George Rokos via Phabricator via Openmp-commits]

grokos added a comment.

In D68100#2173822 <https://reviews.llvm.org/D68100#2173822>, @lildmh wrote:

> This is an optimization brought up by Deepak. I guess you were in that meeting too but forgot. It could be quite useful when you map an array of struct/class. Assume you map 1000 of this structure, with this optimization most memory allocation can be done in a single allocation, instead of allocation 12 bytes memory 1000 times.
>
> Thinking about it, it's actually important for correctness too. Assume you map `C a[2]`. If you map separately, a[0] and a[1] could be mapped to not contiguous locations, and it will cause error/segfault when GPU kernel access this array. If you allocate the whole array `a[2]` together, such problem won't happen.

Sorry for the late response. Here you are talking about something else. The case you are considering is an array of structs. In this case, indeed we have to allocate the whole array beforehand. It's not an optimization, it's a correctness issue as you point out (array objects must be allocated consecutively). In the failing tests, however, we have single structs, not an array of structs. The difference is that in the former case the object we are mapping is the array, whereas in the latter case it's the struct. The two cases are not related to one another unless we intend to treat both of them uniformly, i.e. even if we have a single struct we still treat it as if it were the sole element of a length-1 array. Do I understand correctly?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68100/new/

https://reviews.llvm.org/D68100