[PATCH] D11361: [OpenMP] Target directive host codegen

[Samuel Antao]

sfantao added a comment.

Alexey,

Thanks for the review! I am addressing your remarks as inline comments.

Thanks again,
Samuel


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:188-212
@@ -179,11 +187,27 @@
   };
+
+  /// \brief Values for bit flags used to specify the mapping type for
+  /// offloading.
+  enum OpenMPOffloadMappingFlags {
+    /// \brief Allocate memory on the device and move data from host to device.
+    OMP_MAP_TO = 0x01,
+    /// \brief Allocate memory on the device and move data from device to host.
+    OMP_MAP_FROM = 0x02,
+  };
+
+  enum OpenMPOffloadingReservedDeviceIDs {
+    /// \brief Device ID if the device was not defined, runtime should get it
+    /// from environment variables in the spec.
+    OMP_DEVICEID_UNDEF = -1,
+  };
+
   CodeGenModule &CGM;
   /// \brief Default const ident_t object used for initialization of all other
   /// ident_t objects.
   llvm::Constant *DefaultOpenMPPSource;
   /// \brief Map of flags and corresponding default locations.
   typedef llvm::DenseMap<unsigned, llvm::Value *> OpenMPDefaultLocMapTy;
   OpenMPDefaultLocMapTy OpenMPDefaultLocMap;
   llvm::Value *getOrCreateDefaultLocation(OpenMPLocationFlags Flags);
   /// \brief Describes ident structure that describes a source location.
   /// All descriptions are taken from
----------------
Done!

================
Comment at: lib/CodeGen/CGStmt.cpp:2215-2245
@@ -2213,4 +2214,33 @@
   FunctionArgList Args;
-  Args.append(CD->param_begin(), CD->param_end());
+
+  // If this is an offload function, we need pass a reference to each captured
+  // declarations as arguments.
+  if (isOffloadFunction) {
+    DeclContext *DC = CapturedDecl::castToDeclContext(CD)->getParent();
+    auto ri = RD->field_begin();
+    for (CapturedStmt::const_capture_iterator ci = S.capture_begin(),
+                                              ce = S.capture_end();
+         ci != ce; ++ci, ++ri) {
+      StringRef Name;
+      QualType Ty;
+      if (ci->capturesVariableArrayType()) {
+        Ty = Ctx.getPointerType(ri->getType());
+        Name = "__vla_size";
+      } else if (ci->capturesThis()) {
+        Ty = ri->getType();
+        Name = "__this";
+      } else {
+        const VarDecl *VD = ci->getCapturedVar();
+        Ty = Ctx.getPointerType(VD->getType());
+        Name = VD->getName();
+      }
+
+      IdentifierInfo *ParamName = &Ctx.Idents.get(Name);
+      ImplicitParamDecl *Param =
+          ImplicitParamDecl::Create(Ctx, DC, Loc, ParamName, Ty);
+      Args.push_back(Param);
+    }
+  } else
+    Args.append(CD->param_begin(), CD->param_end());
 
   // Create the function declaration.
----------------
There is only one argument that comes with the CaptureDecl and that is the anonymous struct that captures all the references. 

For the target outlined functions we have to pass each capture separately because not only are they arguments but also data mappings. Therefore we don't have to generate the anonymous struct at all and use the captures directly, signaling the proper map types. This follows what was discussed in the offloading infrastructure proposal - the compiler will generate arrays with each argument/mapping and forward that to the target specific plugin. The target function is therefore expected to have the same signature of the host function that is being generated by this patch, so that the order in the array is consistent with the signature.

The code in this patch that is generating the host version of the target region can therefore be completely reused to generate the device version.


http://reviews.llvm.org/D11361