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

[Alexey Bataev via cfe-commits]

ABataev 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!");
----------------
~0u

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2349
@@ +2348,3 @@
+    TgtBinaryDescriptorTy = llvm::StructType::create(
+        "tgt_bin_desc", CGM.Int32Ty, getTgtDeviceImageTy()->getPointerTo(),
+        getTgtOffloadEntryTy()->getPointerTo(),
----------------
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().

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:325-328
@@ +324,6 @@
+  public:
+    CodeGenModule &CGM;
+
+    /// \brief Number of entries registered so far.
+    unsigned OffloadingEntriesNum;
+
----------------
I think these members must be private.

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

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

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

================
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;
+
----------------
Private/protected

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

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

================
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();
 };
----------------
Do we really need this stuff? CodeGenFunction has member CurFuncDecl, which, it seems, could give you required info without these functions.


http://reviews.llvm.org/D12614