[PATCH] D12614: [OpenMP] Offloading descriptor registration and device codegen.

[Samuel Antao via cfe-commits]

sfantao added inline comments.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1962
@@ +1961,3 @@
+                                            [LineNum][ColNum];
+    assert(Entry.Order != -1u && "Entry not initialized!");
+    assert(!Entry.Addr && !Entry.ID && "Entry registered already!");
----------------
ABataev wrote:
> ~0u
Done.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2349
@@ +2348,3 @@
+    TgtBinaryDescriptorTy = llvm::StructType::create(
+        "tgt_bin_desc", CGM.Int32Ty, getTgtDeviceImageTy()->getPointerTo(),
+        getTgtOffloadEntryTy()->getPointerTo(),
----------------
ABataev wrote:
> I think there should be 4-bytes padding between NumDevices and DeviceImages fields in 64-bit mode, right? It is better to create this structure as clang AST RecordDecl/CXXRecordDecl and then use CGM.getTypes().ConvertTypeForMem().
Done!

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:325-328
@@ +324,6 @@
+  public:
+    CodeGenModule &CGM;
+
+    /// \brief Number of entries registered so far.
+    unsigned OffloadingEntriesNum;
+
----------------
ABataev wrote:
> I think these members must be private.
Done! Also added some setters and getters for the privatized fields.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:339
@@ +338,3 @@
+        // Invalid entry info.
+        OFFLOAD_ENTRY_INFO_INVALID = -1u
+      };
----------------
ABataev wrote:
> Better ~0
Done!

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:343
@@ +342,3 @@
+      // \brief Order this entry was emitted.
+      unsigned Order;
+
----------------
ABataev wrote:
> Make it private/protected also
Done!

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:345
@@ +344,3 @@
+
+      OffloadEntryInfo() : Order(-1u), Kind(OFFLOAD_ENTRY_INFO_INVALID) {}
+      OffloadEntryInfo(OffloadingEntryInfoKinds Kind, unsigned Order)
----------------
ABataev wrote:
> explicit, ~0u
Done

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:367-370
@@ +366,6 @@
+    public:
+      // \brief Address of the entity that has to be mapped for offloading.
+      llvm::Constant *Addr;
+      // \brief Address that can be used as the ID of the entry.
+      llvm::Constant *ID;
+
----------------
ABataev wrote:
> Private/protected
Done

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:372
@@ +371,3 @@
+
+      OffloadEntryInfoTargetRegion()
+          : OffloadEntryInfo(OFFLOAD_ENTRY_INFO_TARGET_REGION, -1u),
----------------
ABataev wrote:
> explicit
Done

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:373
@@ +372,3 @@
+      OffloadEntryInfoTargetRegion()
+          : OffloadEntryInfo(OFFLOAD_ENTRY_INFO_TARGET_REGION, -1u),
+            Addr(nullptr), ID(nullptr) {}
----------------
ABataev wrote:
> ~0u
Done

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:957-967
@@ -759,1 +956,13 @@
+
+  /// \brief Notify the runtime implementation that a user function definition
+  /// is about to be emmited.
+  /// \param GD Declaration of the function definition that will be emitted.
+  void pushCurrentUserFunction(GlobalDecl GD);
+
+  /// \brief Notify the runtime implementation that the emission of the current
+  /// user function is finished.
+  void popCurrentUserFunction();
+
+  /// \brief Get the declaration of the current user function being emitted.
+  StringRef getCurrentUserFunction();
 };
----------------
ABataev wrote:
> Do we really need this stuff? CodeGenFunction has member CurFuncDecl, which, it seems, could give you required info without these functions.
I can't rely on CurFuncDecl because the parent function can be in some cases an implicit outlined function, and what I need is the enclosing user function.

In the new diff, I implemented this in a slightly different way: I forward the user function `GlobalDecl` to the implicit functions (see `GenerateOpenMPCapturedStmtFunction`).

In order for this to work I had to add special login in the scanning of the target regions to deal with lambda functions given that they are also emitted as global definitions. 

Hope you like this approach better.


http://reviews.llvm.org/D12614