[Openmp-commits] [PATCH] D96667: [OpenMP][FIX] Avoid use of stack allocations in asynchronous calls

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added a comment.

In D96667#2562557 <https://reviews.llvm.org/D96667#2562557>, @jdoerfert wrote:

> In D96667#2562413 <https://reviews.llvm.org/D96667#2562413>, @JonChesterfield wrote:
>
>> LG. Why deque over list?
>
> Some small memory overhead sounds better than linear allocation cost.
>
> @tianshilei1992 Can you back port a variation of this to LLVM 12?

D96444 <https://reviews.llvm.org/D96444> can be landed w/o any dependence. This patch can be landed afterwards. Seems we don't need a variant.



================
Comment at: openmp/libomptarget/include/omptarget.h:162
+  /// AsyncInfoTy object. The location can be used as intermediate buffer.
+  void *&getVoidPtrLocation();
 };
----------------
jdoerfert wrote:
> tianshilei1992 wrote:
> > I'd go with a more generic method:
> > ```
> > std::vector<std::unique_ptr<char[]>> BufferLocations;
> > template <typename T>
> > T *getPtr() {
> >   BufferLocations.emplace_back(std::make_unique<char[]>(sizeof(T)));
> >   return reinterpret_cast<T *>(BufferLocations.back().get());
> > }
> > ```
> That seems overkill as long as we only get `void*` out of this. My method avoids one level of indirection and the unique pointer has the same lifetime as the vector anyway.
I'm fine with your method. It's just too "tricky". Having a queue of 4B/8B objects is not so "natural".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96667