[Openmp-commits] [PATCH] D60884: [OpenMP][libomptarget] Enable usage of unified memory for declare target link variables

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added inline comments.


================
Comment at: libomptarget/include/omptargetplugin.h:34-36
+// Initialize the requires flags for the device.
+void __tgt_rtl_init_requires(int64_t RequiresFlags);
+
----------------
According to `init_requires_ty`, this should have a return type. I think it's a good idea to have that from the beginning, this might be useful in the future without breaking compatibility.


================
Comment at: libomptarget/plugins/generic-elf-64bit/src/rtl.cpp:73-75
+  // OpenMP Requires Flags
+  int64_t RequiresFlags;
+
----------------
As before in D60223: You don't need this here and the implementation is optional, so it can go away.


================
Comment at: libomptarget/plugins/generic-elf-64bit/src/rtl.cpp:155-159
+void __tgt_rtl_init_requires(int64_t RequiresFlags) {
+  DP("Init requires flags to %ld\n", RequiresFlags);
+  DeviceInfo.RequiresFlags = RequiresFlags;
+}
+
----------------
likewise


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D60884