[Openmp-commits] [PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records

Alexey Bataev via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Oct 11 12:09:36 PDT 2018

ABataev added a subscriber: Hahnfeld.
ABataev added a comment.

In https://reviews.llvm.org/D53141#1262378, @Hahnfeld wrote:

> In https://reviews.llvm.org/D53141#1262019, @ABataev wrote:
> > In https://reviews.llvm.org/D53141#1261996, @Hahnfeld wrote:
> >
> > > I guess this will break the case when the `DataSize` passed to `__kmpc_data_sharing_push_stack()` needs additional alignment: With this change it is handled in `data_sharing_push_stack_common()` but `__kmpc_data_sharing_push_stack()` will determine `PushSize` without the adjustment and do the final pointer arithmetic.
> >
> >
> > Why `DataSize` might require an additional alignment? The `DataSize` must be already aligned.
> It's required in `__kmpc_data_sharing_push_stack()` for some cases, please see https://reviews.llvm.org/D52655.

It is another problem. The problem is that the `StackP` pointer is unaligned after all the pointer maths. The DataSize itself should not be aligned.

> Another problem with this patch is that `__kmpc_data_sharing_push_stack()` is adding the lane offset to the return value of `omptarget_nvptx_SimpleThreadPrivateContext::Allocate()`. AFAICS this will break `lastprivate` which requires the same buffer for all threads.
> Anyway, you must do what you think to be correct.

Yes, there must be an additional check `size_t PushSize = (isRuntimeUninitialized() || IsMasterThread()) ? DataSize : WARPSIZE * DataSize;`

  rOMP OpenMP


More information about the Openmp-commits mailing list