[Openmp-commits] [PATCH] D51875: [OPENMP][NVPTX] Add support for lastprivates/reductions handling in SPMD constructs with lightweight runtime.

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D51875#1229565, @ABataev wrote:

> In https://reviews.llvm.org/D51875#1229545, @Hahnfeld wrote:
>
> > In https://reviews.llvm.org/D51875#1229536, @ABataev wrote:
> >
> > > I already described it - it breaks the compatibility with other outlined regions and breaks the whole design of the OpenMP implementation.
> >
> >
> > First that's a general statement without any explanation. Second I'm not asking about the scratchpad pointer solution in ibm-devel but rather why we can't pass `RequiresDataSharing = true` to `__kmpc_spmd_kernel_init`. Which will give us the data sharing in existing buffers.
>
>
> First, stop talking like this. I don't owe you anything.


Sorry, my last comment sounds rude even though I didn't mean it.
My point is that it's impossible to review patches without a big picture: what are the other parts, which alternatives did you evaluate, why don't they work?
And to be honest: Disregarding technical review and simply ignoring my comments doesn't feel nice either.

> Second, `RequiresDataSharing` is not required, because I tend to use the preallocated buffer instead of dynamically allocated.

The data sharing infrastructure also has preallocated buffers.

> 
> 
>> 
>> 
>>> 
>>> 
>>>>> I'd like to have this at least as a temporirily solution to support lastprivates/reductions in SPMD mode with lightweight runtime. We can reduce the size of the preallocated buffers, if you wish.
>>>> 
>>>> Is that a commitment to actively work on that area?
>>> 
>>> Yes, Alex Eichenberger tries to invent something, that will allow us to use something similar to ibm-devel but without breaking the design of OpenMP in the compiler. But it requires some time. But I'd like to have something working, at least.
>> 
>> I'm referring to the process of cleaning up `libomptarget-nvptx`.
> 
> No, if you're interested in this, you can do it.

That's what I feared. Yes, I think this is needed, but to be honest, I'm already facing enough resistence with moderately conservative proposals.

In https://reviews.llvm.org/D51875#1229566, @ABataev wrote:

> No, it is about 10-12 Mb.


The additional buffer, maybe. But raising `MAX_SM` from 56 to 84 scales the array of queues proportionally.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D51875