[Openmp-commits] [PATCH] D43626: [OpenMP] Remove implicit data sharing using device shared memory from libomptarget

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D43626#1027103, @gtbercea wrote:

> In https://reviews.llvm.org/D43626#1025517, @Hahnfeld wrote:
>
> > In https://reviews.llvm.org/D43626#1024135, @grokos wrote:
> >
> > > @Hahnfeld: Did our private discussion answer your questions about this plan?
> >
> >
> > Partly, I'm still not happy about this turns out to be required after all of this was upstreamed. However if it's really needed you should at least make sure that basic applications continue to compile, something which we only reached recently.
>
>
> Data sharing cannot be used today due to the missing LLVM NVPTX backend patch so it should not affect applications that compile right now. This patch and it's Clang counterpart need to be committed at the same time to ensure continuity.


Got that but that doesn't justify upstreaming a part and kind of reverting it a few weeks later.

>> In any case, the result of our private discussion needs to be made public. In particular I still don't see either explanation nor reasoning in any of the patches.
> 
> The shared memory scheme was never meant to cover all the cases in OpenMP and it was meant more as an optimization. For that to be true, the global memory scheme needs to be the default scheme used. The shared memory scheme can then be re-intorduced on top of the global memory scheme as an optimization.

Again: You should put that in the summary of all patches!


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D43626