[Openmp-commits] [PATCH] D81054: [OpenMP] Introduce target memory manager

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/src/MemoryManager.cpp:324
+  if (Threshold)
+    SizeThreshold = Threshold;
+}
----------------
ye-luo wrote:
> tianshilei1992 wrote:
> > ye-luo wrote:
> > > SizeThreshold is global while Threshold is local. The default values is also different. I'm lost in the logic here.
> > Yeah, you're lost. By default, `Threshold` is 0, which means we will not overwrite `SizeThreshold`.
> Q1. Why SizeThreshold is not per device?
> Q2. I was asking for a way to opt-out this optimization. But you ignore LIBOMPTARGET_MEMORY_MANAGER_THRESHOLD=0
Remove Q2. Opt-out has been supported.


================
Comment at: openmp/libomptarget/src/MemoryManager.h:30
+  /// Constructor
+  MemoryManagerTy(DeviceTy &D, size_t Threshold = 0);
+
----------------
Second (Third?) place with a default. Remove or error out if size 0?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81054