[PATCH] D43026: [OpenMP] Support for implicit "declare target" functions - CodeGen patch

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

It is impossible to understand what is going on here. We need to discuss this before even reviewing of this patch.



================
Comment at: lib/CodeGen/CGCXXABI.h:543
+                               llvm::GlobalVariable *DeclPtr, bool PerformInit,
+                               bool EmitInitOnly, bool EmitDtorOnly) = 0;
 
----------------
Why do you need these additional parameters?


================
Comment at: lib/CodeGen/CGDeclCXX.cpp:153-154
+                                               bool PerformInit,
+                                               bool EmitInitOnly,
+                                               bool EmitDtorOnly) {
 
----------------
Why do you need these parameters?


================
Comment at: lib/CodeGen/CGDeclCXX.cpp:588-590
+  if (CGM.getLangOpts().OpenMP)
+    CGM.getOpenMPRuntime().registerDeviceCtorDtorLaunching(*this, *D, Addr,
+                                                           PerformInit);
----------------
1. Check that this is the device codegen.
2. Enclose in braces


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3484-3486
+  if (Entry->second.isRegistered())
+    return false;
+  return true;
----------------
Just `return !Entry->second.isRegistered();`


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:388
 
-    /// \brief Number of entries registered so far.
-    unsigned OffloadingEntriesNum;
+    /// \brief Number of ordered entries registered so far.
+    unsigned OffloadingOrderedEntriesNum = 0u;
----------------
Remove `brief`s


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:508-509
+      OffloadEntryInfoDeviceGlobalVar()
+          : OffloadEntryInfo(OFFLOAD_ENTRY_INFO_DEVICE_GLOBAL_VAR, ~0u,
+                             /*Flags=*/0u),
+            Addr(nullptr) {}
----------------
Use enumeric values rather than the magical numbers.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:559-563
+          : OffloadEntryInfo(OFFLOAD_ENTRY_INFO_DEVICE_FUNCTION, ~0u,
+                             /*Flags=*/0u) {}
+      explicit OffloadEntryInfoDeviceFunction(bool IsRegistered)
+          : OffloadEntryInfo(OFFLOAD_ENTRY_INFO_DEVICE_FUNCTION, ~0u,
+                             /*Flags=*/0u),
----------------
Use enum constants rather than the magical numbers


Repository:
  rC Clang

https://reviews.llvm.org/D43026