Skip to content

[Offload] Remove remaining __tgt_register_requires references #90198

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

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Apr 26, 2024

Summary:
This call was removed a few months ago to allow the runtime to actually
init / deinit in a correct order. However that patch forgot to remove a
few leftover uses.

@llvmbot llvmbot added flang:openmp clang:openmp OpenMP related changes to Clang offload labels Apr 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2024

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-offload

@llvm/pr-subscribers-flang-openmp

Author: Joseph Huber (jhuber6)

Changes

Summary:
This call was removed a few months ago to allow the runtime to actually
init / deinit in a correct order. However that patch forgot to remove a
few leftover uses.


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

4 Files Affected:

  • (modified) llvm/include/llvm/Frontend/OpenMP/OMPKinds.def (-3)
  • (modified) offload/test/unified_shared_memory/api.c (-9)
  • (modified) offload/test/unified_shared_memory/close_manual.c (-6)
  • (modified) offload/test/unified_shared_memory/shared_update.c (-9)
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def b/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
index d22d2a8e948b00..fe09bb8177c28e 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -436,7 +436,6 @@ __OMP_RTL(__tgt_target_kernel, false, Int32, IdentPtr, Int64, Int32, Int32,
           VoidPtr, KernelArgsPtr)
 __OMP_RTL(__tgt_target_kernel_nowait, false, Int32, IdentPtr, Int64, Int32,
           Int32, VoidPtr, KernelArgsPtr, Int32, VoidPtr, Int32, VoidPtr)
-__OMP_RTL(__tgt_register_requires, false, Void, Int64)
 __OMP_RTL(__tgt_target_data_begin_mapper, false, Void, IdentPtr, Int64, Int32, VoidPtrPtr,
           VoidPtrPtr, Int64Ptr, Int64Ptr, VoidPtrPtr, VoidPtrPtr)
 __OMP_RTL(__tgt_target_data_begin_nowait_mapper, false, Void, IdentPtr, Int64, Int32,
@@ -1025,8 +1024,6 @@ __OMP_RTL_ATTRS(__tgt_target_kernel_nowait, ForkAttrs, SExt,
                 ParamAttrs(AttributeSet(), AttributeSet(), SExt, SExt,
                            AttributeSet(), AttributeSet(), SExt, AttributeSet(),
                            SExt))
-__OMP_RTL_ATTRS(__tgt_register_requires, ForkAttrs, AttributeSet(),
-                ParamAttrs())
 __OMP_RTL_ATTRS(__tgt_target_data_begin_mapper, ForkAttrs, AttributeSet(),
                 ParamAttrs(AttributeSet(), AttributeSet(), SExt))
 __OMP_RTL_ATTRS(__tgt_target_data_begin_nowait_mapper, ForkAttrs, AttributeSet(),
diff --git a/offload/test/unified_shared_memory/api.c b/offload/test/unified_shared_memory/api.c
index c7ab055abb5152..b938971b4b03cf 100644
--- a/offload/test/unified_shared_memory/api.c
+++ b/offload/test/unified_shared_memory/api.c
@@ -9,11 +9,6 @@
 #include <omp.h>
 #include <stdio.h>
 
-// ---------------------------------------------------------------------------
-// Various definitions copied from OpenMP RTL
-
-extern void __tgt_register_requires(int64_t);
-
 // End of definitions copied from OpenMP RTL.
 // ---------------------------------------------------------------------------
 
@@ -32,10 +27,6 @@ void init(int A[], int B[], int C[]) {
 int main(int argc, char *argv[]) {
   const int device = omp_get_default_device();
 
-  // Manual registration of requires flags for Clang versions
-  // that do not support requires.
-  __tgt_register_requires(8);
-
   // CHECK: Initial device: [[INITIAL_DEVICE:[0-9]+]]
   printf("Initial device: %d\n", omp_get_initial_device());
   // CHECK: Num devices: [[INITIAL_DEVICE]]
diff --git a/offload/test/unified_shared_memory/close_manual.c b/offload/test/unified_shared_memory/close_manual.c
index 9985e822c05d7a..c588cb1c403a7c 100644
--- a/offload/test/unified_shared_memory/close_manual.c
+++ b/offload/test/unified_shared_memory/close_manual.c
@@ -8,8 +8,6 @@
 // ---------------------------------------------------------------------------
 // Various definitions copied from OpenMP RTL
 
-extern void __tgt_register_requires(int64_t);
-
 extern void __tgt_target_data_begin(int64_t device_id, int32_t arg_num,
                                     void **args_base, void **args,
                                     int64_t *arg_sizes, int64_t *arg_types);
@@ -30,10 +28,6 @@ int main(int argc, char *argv[]) {
   void *host_alloc = 0, *device_alloc = 0;
   int *a = (int *)malloc(N * sizeof(int));
 
-  // Manual registration of requires flags for Clang versions
-  // that do not support requires.
-  __tgt_register_requires(8);
-
   // Init
   for (int i = 0; i < N; ++i) {
     a[i] = 10;
diff --git a/offload/test/unified_shared_memory/shared_update.c b/offload/test/unified_shared_memory/shared_update.c
index 65db9e4f6bdce2..f8eb11d56a6cad 100644
--- a/offload/test/unified_shared_memory/shared_update.c
+++ b/offload/test/unified_shared_memory/shared_update.c
@@ -11,11 +11,6 @@
 #include <omp.h>
 #include <stdio.h>
 
-// ---------------------------------------------------------------------------
-// Various definitions copied from OpenMP RTL
-
-extern void __tgt_register_requires(int64_t);
-
 // End of definitions copied from OpenMP RTL.
 // ---------------------------------------------------------------------------
 
@@ -30,10 +25,6 @@ int main(int argc, char *argv[]) {
   int *alloc = (int *)malloc(N * sizeof(int));
   int data[N];
 
-  // Manual registration of requires flags for Clang versions
-  // that do not support requires.
-  __tgt_register_requires(8);
-
   for (int i = 0; i < N; ++i) {
     alloc[i] = 10;
     data[i] = 1;

@shiltian
Copy link
Contributor

surprised this was not caught by those tests

Summary:
This call was removed a few months ago to allow the runtime to actually
init / deinit in a correct order. However that patch forgot to remove a
few leftover uses.
@jhuber6
Copy link
Contributor Author

jhuber6 commented Apr 26, 2024

surprised this was not caught by those tests

I think it printed a warning but it was effectively a no-op.

Copy link
Contributor

@jplehr jplehr left a comment

Choose a reason for hiding this comment

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

I agree it's odd that these tests did not catch it.
I just verified that they are not marked as UNSUPPORTED in the offload bot.

@jhuber6 jhuber6 merged commit 904b1a8 into llvm:main Apr 26, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants