[Openmp-commits] [PATCH] D63106: [OpenMP][libomptarget] Add support for declare target to clause under unified memory

Gheorghe-Teodor Bercea via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Fri Jun 14 11:24:09 PDT 2019

gtbercea marked an inline comment as done.
gtbercea added inline comments.

Comment at: libomptarget/plugins/cuda/src/rtl.cpp:452-453
       if (DeviceInfo.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
-          e->flags & OMP_DECLARE_TARGET_LINK) {
+          (e->flags & OMP_DECLARE_TARGET_LINK ||
+           e->flags == OMP_DECLARE_TARGET_TO)) {
         // If unified memory is present any target link variables
grokos wrote:
> Maybe this OR is redundant; it always evaluates to true. Since the symbol we are processing has size!=0 it is a variable and variables are always either "to" or "link", there are no other possibilities (at least now, maybe in the future more attributes will be added).
So I could remove the attribute check and just have:

if (DeviceInfo.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) {..} 
Would that work?

The reason I included this explicit check (even if it evaluates to true always) is that if in the future this is extended with other attributes then this check will not automatically work for that. I think this disambiguates the cases that are supported today and guard against this path being taken unintentionally.

  rOMP OpenMP



More information about the Openmp-commits mailing list