[Openmp-commits] [PATCH] D44470: [OpenMP][libomptarget] Enable multiple frames per global memory slot

[George Rokos via Phabricator via Openmp-commits]

grokos added inline comments.


================
Comment at: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu:400
+      // Check if there is a next slot.
+      if (__kmpc_data_sharing_slot *ExistingSlot = SlotP->Next) {
+        // Attempt to reuse an existing slot provided the data fits in the slot.
----------------
gtbercea wrote:
> grokos wrote:
> > So here we are only inspecting the very next slot hoping that it will be large enough to accommodate our request. In case the next slot is not big enough, could there be slots after the next which are suitable? If this scenario is possible, then why are we only inspecting the very next slot and delete everything thereafter if it's not big enough?
> It's definitely do-able but chances of that logic being applied are small. We expect the vast majority of requests to fit the default size of the the slot. For data that is larger than the default case it is vastly more likely that there's no next slot and we just create an entirely new slot.
> There is also the issue of space: keeping around a list that occasionally has completely empty slots will be wasteful (you could argue that since it's a linked list we can just delete those nodes and redo the links). What I can do is as I delete the "next" node I can check if the data fits, if it does I can just place it there.
Yes, that's what I had in mind. We are traversing the list anyway, so checking before deleting comes for free - it doesn't raise the complexity.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D44470