Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OpenMP][NFC] Create an "OpenMP" folder in the include folder #73713

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

jdoerfert
Copy link
Member

Not everything in libomptarget (include) is "OpenMP", but some things most certainly are. This commit moves some code around to start making this distinction without the intention to change functionality.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-openmp

Author: Johannes Doerfert (jdoerfert)

Changes

Not everything in libomptarget (include) is "OpenMP", but some things most certainly are. This commit moves some code around to start making this distinction without the intention to change functionality.


Full diff: https://github.com/llvm/llvm-project/pull/73713.diff

7 Files Affected:

  • (added) openmp/libomptarget/include/OpenMP/InternalTypes.h (+32)
  • (renamed) openmp/libomptarget/include/OpenMP/InteropAPI.h (+8-10)
  • (added) openmp/libomptarget/include/OpenMP/omp.h (+23)
  • (modified) openmp/libomptarget/src/CMakeLists.txt (+1-1)
  • (renamed) openmp/libomptarget/src/OpenMP/InteropAPI.cpp (+33-24)
  • (modified) openmp/libomptarget/src/omptarget.cpp (+2)
  • (modified) openmp/libomptarget/src/private.h (+2-17)
diff --git a/openmp/libomptarget/include/OpenMP/InternalTypes.h b/openmp/libomptarget/include/OpenMP/InternalTypes.h
new file mode 100644
index 000000000000000..5b1f2026f324496
--- /dev/null
+++ b/openmp/libomptarget/include/OpenMP/InternalTypes.h
@@ -0,0 +1,32 @@
+//===-- OpenMP/InternalTypes.h -- Internal OpenMP Types ------------- C++ -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Private type declarations and helper macros for OpenMP.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef OMPTARGET_OPENMP_INTERNAL_TYPES_H
+#define OMPTARGET_OPENMP_INTERNAL_TYPES_H
+
+#include <cstddef>
+#include <cstdint>
+
+typedef intptr_t kmp_intptr_t;
+
+// Compiler sends us this info:
+typedef struct kmp_depend_info {
+  kmp_intptr_t base_addr;
+  size_t len;
+  struct {
+    bool in : 1;
+    bool out : 1;
+    bool mtx : 1;
+  } flags;
+} kmp_depend_info_t;
+
+#endif // OMPTARGET_OPENMP_INTERNAL_TYPES_H
diff --git a/openmp/libomptarget/include/interop.h b/openmp/libomptarget/include/OpenMP/InteropAPI.h
similarity index 95%
rename from openmp/libomptarget/include/interop.h
rename to openmp/libomptarget/include/OpenMP/InteropAPI.h
index ed3aa0d83a8637b..5781264817de330 100644
--- a/openmp/libomptarget/include/interop.h
+++ b/openmp/libomptarget/include/OpenMP/InteropAPI.h
@@ -1,16 +1,17 @@
-//===----------------------------------------------------------------------===//
+//===-- OpenMP/InteropAPI.h - OpenMP interoperability types and API - C++ -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
+//
+//===----------------------------------------------------------------------===//
 
-#ifndef _INTEROP_H_
-#define _INTEROP_H_
+#ifndef OMPTARGET_OPENMP_INTEROP_API_H
+#define OMPTARGET_OPENMP_INTEROP_API_H
 
 #include "omptarget.h"
-#include <assert.h>
 
 #if defined(_WIN32)
 #define __KAI_KMPC_CONVENTION __cdecl
@@ -24,9 +25,7 @@
 #endif
 #endif
 
-#ifdef __cplusplus
 extern "C" {
-#endif
 
 /// TODO: Include the `omp.h` of the current build
 /* OpenMP 5.1 interop */
@@ -151,7 +150,6 @@ typedef struct omp_interop_val_t {
   const intptr_t backend_type_id = omp_interop_backend_type_cuda_1;
 } omp_interop_val_t;
 
-#ifdef __cplusplus
-}
-#endif
-#endif
+} // extern "C"
+
+#endif // OMPTARGET_OPENMP_INTEROP_API_H
diff --git a/openmp/libomptarget/include/OpenMP/omp.h b/openmp/libomptarget/include/OpenMP/omp.h
new file mode 100644
index 000000000000000..e2d18819878a7f4
--- /dev/null
+++ b/openmp/libomptarget/include/OpenMP/omp.h
@@ -0,0 +1,23 @@
+//===-- OpenMP/omp.h - Copies of OpenMP user facing types and APIs - C++ -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This copies some OpenMP user facing types and APIs for easy reach within the
+// implementation.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef OMPTARGET_OPENMP_OMP_H
+#define OMPTARGET_OPENMP_OMP_H
+
+extern "C" {
+
+int omp_get_default_device(void) __attribute__((weak));
+
+} // extern "C"
+
+#endif // OMPTARGET_OPENMP_OMP_H
diff --git a/openmp/libomptarget/src/CMakeLists.txt b/openmp/libomptarget/src/CMakeLists.txt
index 34a8273cfaf645c..4ef0b8124acdb43 100644
--- a/openmp/libomptarget/src/CMakeLists.txt
+++ b/openmp/libomptarget/src/CMakeLists.txt
@@ -18,11 +18,11 @@ add_llvm_library(omptarget
   api.cpp
   device.cpp
   interface.cpp
-  interop.cpp
   omptarget.cpp
   OmptCallback.cpp
   rtl.cpp
   LegacyAPI.cpp
+  OpenMP/InteropAPI.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LIBOMPTARGET_INCLUDE_DIR}
diff --git a/openmp/libomptarget/src/interop.cpp b/openmp/libomptarget/src/OpenMP/InteropAPI.cpp
similarity index 86%
rename from openmp/libomptarget/src/interop.cpp
rename to openmp/libomptarget/src/OpenMP/InteropAPI.cpp
index 0f6c887060ee54c..ace0fecd0a43624 100644
--- a/openmp/libomptarget/src/interop.cpp
+++ b/openmp/libomptarget/src/OpenMP/InteropAPI.cpp
@@ -1,4 +1,4 @@
-//===---------------interop.cpp - Implementation of interop directive -----===//
+//===-- InteropAPI.cpp - Implementation of OpenMP interoperability API ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,21 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "interop.h"
-#include "private.h"
+#include "OpenMP/InteropAPI.h"
+#include "OpenMP/InternalTypes.h"
+#include "OpenMP/omp.h"
+
+#include "device.h"
+#include "omptarget.h"
+
+extern "C" {
+
+void __kmpc_omp_wait_deps(ident_t *loc_ref, int32_t gtid, int32_t ndeps,
+                          kmp_depend_info_t *dep_list, int32_t ndeps_noalias,
+                          kmp_depend_info_t *noalias_dep_list)
+    __attribute__((weak));
+
+} // extern "C"
 
 namespace {
 omp_interop_rc_t getPropertyErrorType(omp_interop_property_t Property) {
@@ -176,17 +189,14 @@ __OMP_GET_INTEROP_TY3(const char *, type_desc)
 __OMP_GET_INTEROP_TY3(const char *, rc_desc)
 #undef __OMP_GET_INTEROP_TY3
 
-typedef int64_t kmp_int64;
-
-#ifdef __cplusplus
 extern "C" {
-#endif
-void __tgt_interop_init(ident_t *LocRef, kmp_int32 Gtid,
+
+void __tgt_interop_init(ident_t *LocRef, int32_t Gtid,
                         omp_interop_val_t *&InteropPtr,
-                        kmp_interop_type_t InteropType, kmp_int32 DeviceId,
-                        kmp_int32 Ndeps, kmp_depend_info_t *DepList,
-                        kmp_int32 HaveNowait) {
-  kmp_int32 NdepsNoalias = 0;
+                        kmp_interop_type_t InteropType, int32_t DeviceId,
+                        int32_t Ndeps, kmp_depend_info_t *DepList,
+                        int32_t HaveNowait) {
+  int32_t NdepsNoalias = 0;
   kmp_depend_info_t *NoaliasDepList = NULL;
   assert(InteropType != kmp_interop_type_unknown &&
          "Cannot initialize with unknown interop_type!");
@@ -221,11 +231,11 @@ void __tgt_interop_init(ident_t *LocRef, kmp_int32 Gtid,
   }
 }
 
-void __tgt_interop_use(ident_t *LocRef, kmp_int32 Gtid,
-                       omp_interop_val_t *&InteropPtr, kmp_int32 DeviceId,
-                       kmp_int32 Ndeps, kmp_depend_info_t *DepList,
-                       kmp_int32 HaveNowait) {
-  kmp_int32 NdepsNoalias = 0;
+void __tgt_interop_use(ident_t *LocRef, int32_t Gtid,
+                       omp_interop_val_t *&InteropPtr, int32_t DeviceId,
+                       int32_t Ndeps, kmp_depend_info_t *DepList,
+                       int32_t HaveNowait) {
+  int32_t NdepsNoalias = 0;
   kmp_depend_info_t *NoaliasDepList = NULL;
   assert(InteropPtr && "Cannot use nullptr!");
   omp_interop_val_t *InteropVal = InteropPtr;
@@ -249,11 +259,11 @@ void __tgt_interop_use(ident_t *LocRef, kmp_int32 Gtid,
   // TODO Flush the queue associated with the interop through the plugin
 }
 
-void __tgt_interop_destroy(ident_t *LocRef, kmp_int32 Gtid,
-                           omp_interop_val_t *&InteropPtr, kmp_int32 DeviceId,
-                           kmp_int32 Ndeps, kmp_depend_info_t *DepList,
-                           kmp_int32 HaveNowait) {
-  kmp_int32 NdepsNoalias = 0;
+void __tgt_interop_destroy(ident_t *LocRef, int32_t Gtid,
+                           omp_interop_val_t *&InteropPtr, int32_t DeviceId,
+                           int32_t Ndeps, kmp_depend_info_t *DepList,
+                           int32_t HaveNowait) {
+  int32_t NdepsNoalias = 0;
   kmp_depend_info_t *NoaliasDepList = NULL;
   assert(InteropPtr && "Cannot use nullptr!");
   omp_interop_val_t *InteropVal = InteropPtr;
@@ -281,6 +291,5 @@ void __tgt_interop_destroy(ident_t *LocRef, kmp_int32 Gtid,
   delete InteropPtr;
   InteropPtr = omp_interop_none;
 }
-#ifdef __cplusplus
+
 } // extern "C"
-#endif
diff --git a/openmp/libomptarget/src/omptarget.cpp b/openmp/libomptarget/src/omptarget.cpp
index 318fd77920f6f0e..d3b299ca58f5172 100644
--- a/openmp/libomptarget/src/omptarget.cpp
+++ b/openmp/libomptarget/src/omptarget.cpp
@@ -18,6 +18,8 @@
 #include "private.h"
 #include "rtl.h"
 
+#include "OpenMP/omp.h"
+
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/bit.h"
 
diff --git a/openmp/libomptarget/src/private.h b/openmp/libomptarget/src/private.h
index 37dae0a9f6bea2f..f082f6e3b9fc83a 100644
--- a/openmp/libomptarget/src/private.h
+++ b/openmp/libomptarget/src/private.h
@@ -16,6 +16,8 @@
 #include "Shared/Debug.h"
 #include "Shared/SourceInfo.h"
 
+#include "OpenMP/InternalTypes.h"
+
 #include "device.h"
 #include "omptarget.h"
 
@@ -110,7 +112,6 @@ extern "C" {
  */
 typedef int kmp_int32;
 typedef int64_t kmp_int64;
-typedef intptr_t kmp_intptr_t;
 
 typedef void *omp_depend_t;
 struct kmp_task;
@@ -154,24 +155,8 @@ typedef struct kmp_tasking_flags { /* Total struct must be exactly 32 bits */
   unsigned reserved31 : 7; /* reserved for library use */
 } kmp_tasking_flags_t;
 
-// Compiler sends us this info:
-typedef struct kmp_depend_info {
-  kmp_intptr_t base_addr;
-  size_t len;
-  struct {
-    bool in : 1;
-    bool out : 1;
-    bool mtx : 1;
-  } flags;
-} kmp_depend_info_t;
-// functions that extract info from libomp; keep in sync
-int omp_get_default_device(void) __attribute__((weak));
 int32_t __kmpc_global_thread_num(void *) __attribute__((weak));
 int __kmpc_get_target_offload(void) __attribute__((weak));
-void __kmpc_omp_wait_deps(ident_t *loc_ref, kmp_int32 gtid, kmp_int32 ndeps,
-                          kmp_depend_info_t *dep_list, kmp_int32 ndeps_noalias,
-                          kmp_depend_info_t *noalias_dep_list)
-    __attribute__((weak));
 void **__kmpc_omp_get_target_async_handle_ptr(kmp_int32 gtid)
     __attribute__((weak));
 bool __kmpc_omp_has_task_team(kmp_int32 gtid) __attribute__((weak));

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. Long term I'm wondering if it would be feasible to have a more structured approach to language specific extensions.

Not everything in libomptarget is "OpenMP", but some things most
certainly are. This commit moves some code around to start making this
distinction without the intention to change functionality.
Copy link

github-actions bot commented Nov 28, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff e7f5d609dd7ec73f9e098532d3fa73b9fa4be02d 346bd0733b5200271c2decaf6ade9b86bb00ba80 -- openmp/libomptarget/include/OpenMP/InternalTypes.h openmp/libomptarget/include/OpenMP/InteropAPI.h openmp/libomptarget/src/omptarget.cpp openmp/libomptarget/src/private.h openmp/libomptarget/include/OpenMP/omp.h openmp/libomptarget/src/OpenMP/InteropAPI.cpp
View the diff from clang-format here.
diff --git a/openmp/libomptarget/include/OpenMP/omp.h b/openmp/libomptarget/include/OpenMP/omp.h
index c089667732..78a7c767c3 100644
--- a/openmp/libomptarget/include/OpenMP/omp.h
+++ b/openmp/libomptarget/include/OpenMP/omp.h
@@ -89,8 +89,9 @@ int __KAI_KMPC_CONVENTION omp_get_num_interop_properties(const omp_interop_t);
  * The `omp_get_interop_int` routine retrieves an integer property from an
  * `omp_interop_t` object.
  */
-omp_intptr_t __KAI_KMPC_CONVENTION
-omp_get_interop_int(const omp_interop_t, omp_interop_property_t, int *);
+omp_intptr_t __KAI_KMPC_CONVENTION omp_get_interop_int(const omp_interop_t,
+                                                       omp_interop_property_t,
+                                                       int *);
 /*!
  * The `omp_get_interop_ptr` routine retrieves a pointer property from an
  * `omp_interop_t` object.
@@ -101,8 +102,9 @@ void *__KAI_KMPC_CONVENTION omp_get_interop_ptr(const omp_interop_t,
  * The `omp_get_interop_str` routine retrieves a string property from an
  * `omp_interop_t` object.
  */
-const char *__KAI_KMPC_CONVENTION
-omp_get_interop_str(const omp_interop_t, omp_interop_property_t, int *);
+const char *__KAI_KMPC_CONVENTION omp_get_interop_str(const omp_interop_t,
+                                                      omp_interop_property_t,
+                                                      int *);
 /*!
  * The `omp_get_interop_name` routine retrieves a property name from an
  * `omp_interop_t` object.

@jdoerfert
Copy link
Member Author

Format error is on an outdate version, or at least I can't reproduce it locally.

@jdoerfert jdoerfert merged commit d105701 into llvm:main Nov 28, 2023
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomptarget OpenMP offload runtime openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants