[OpenMP][OMPT] Change OMPT kind for OpenMP test lock functions

[Thyre]

Summary

The OpenMP specification mentions that omp_test_lock and
omp_test_nest_lock dispatch OMPT callbacks with ompt_mutex_test_lock
and ompt_mutex_test_nest_lock for their kind respectively. Previously,
the values ompt_mutex_lock and ompt_mutex_nest_lock were used. This
could cause issues in application relying on the kind to correctly
determine lock states. This commit changes the kind to the expected
ones.

Fixes issue #63261

Depends on D153031

[protze.joachim]

The whole stack looks good to me. My only concern is that with separate patches, the first two patches will lead to failing test pipelines.

[Thyre]

In D153032#4447949, @protze.joachim wrote:

The whole stack looks good to me. My only concern is that with separate patches, the first two patches will lead to failing test pipelines.

I can see that, yeah.
From my perspective, it would make sense if D153028 causes a failing pipeline since we are changing a test case which was incorrect before. The second patch (D153031) doesn't fix the issue, but is important to allow the test to pass in the third patch. It's unfortunate that this patch will still cause the pipeline to fail.

I guess there are three options on how we can handle this:

1. Keep the patches as they are right now
2. Rebase the patches to include D153031 in either the first or third patch
3. Squash all patches down to a single patch

I would tend to the first or second option. For the second option, I would include it in the bug fix itself to keep the test case a non-functional change.

[protze.joachim]

I suggest to squash all 3 patches into a single commit and reference all 3 reviews in this commit. This would also make a possible revert/cherry-pick easier.

[Thyre]

In D153032#4448299, @protze.joachim wrote:

I suggest to squash all 3 patches into a single commit and reference all 3 reviews in this commit. This would also make a possible revert/cherry-pick easier.

Sounds good.
I'm not able to commit the changes directly since this is my first time actually contributing to LLVM (aside from creating issues). I would therefore kindly ask if someone could commit the changes on my behalf.
I guess for the commit message, the text of this patch should be sufficient, if we add the other reviews.

[protze.joachim]

Looks like a single commit cannot close multiple reviews.

openmp/runtime/src/kmp_csupport.cpp

 #endif
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   // This is the case, if called from omp_init_lock_with_hint:
   void *codeptr = OMPT_LOAD_RETURN_ADDRESS(gtid);
   if (!codeptr)
     codeptr = OMPT_GET_RETURN_ADDRESS(0);
   if (ompt_enabled.ompt_callback_mutex_acquire) {
     ompt_callbacks.ompt_callback(ompt_callback_mutex_acquire)(
-        ompt_mutex_lock, omp_lock_hint_none,
+        ompt_mutex_test_lock, omp_lock_hint_none,
         __ompt_get_mutex_impl_type(user_lock),
         (ompt_wait_id_t)(uintptr_t)user_lock, codeptr);
   }
 #endif
 #if KMP_USE_INLINED_TAS
   if (tag == locktag_tas && !__kmp_env_consistency_check) {
     KMP_TEST_TAS_LOCK(user_lock, gtid, rc);
   } else
 #elif KMP_USE_INLINED_FUTEX
   if (tag == locktag_futex && !__kmp_env_consistency_check) {
     KMP_TEST_FUTEX_LOCK(user_lock, gtid, rc);
   } else
 #endif
   {
     rc = __kmp_direct_test[tag]((kmp_dyna_lock_t *)user_lock, gtid);
   }
   if (rc) {
 #if USE_ITT_BUILD
     __kmp_itt_lock_acquired((kmp_user_lock_p)user_lock);
 #endif
 #if OMPT_SUPPORT && OMPT_OPTIONAL
     if (ompt_enabled.ompt_callback_mutex_acquired) {
       ompt_callbacks.ompt_callback(ompt_callback_mutex_acquired)(
-          ompt_mutex_lock, (ompt_wait_id_t)(uintptr_t)user_lock, codeptr);
+          ompt_mutex_test_lock, (ompt_wait_id_t)(uintptr_t)user_lock, codeptr);
     }
 #endif
     return FTN_TRUE;
   } else {
 #if USE_ITT_BUILD
     __kmp_itt_lock_cancelled((kmp_user_lock_p)user_lock);
 #endif
     return FTN_FALSE;

 #endif /* USE_ITT_BUILD */
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   // This is the case, if called from omp_init_lock_with_hint:
   void *codeptr = OMPT_LOAD_RETURN_ADDRESS(gtid);
   if (!codeptr)
     codeptr = OMPT_GET_RETURN_ADDRESS(0);
   if (ompt_enabled.ompt_callback_mutex_acquire) {
     ompt_callbacks.ompt_callback(ompt_callback_mutex_acquire)(
-        ompt_mutex_lock, omp_lock_hint_none, __ompt_get_mutex_impl_type(),
+        ompt_mutex_test_lock, omp_lock_hint_none, __ompt_get_mutex_impl_type(),
         (ompt_wait_id_t)(uintptr_t)lck, codeptr);
   }
 #endif
 
   rc = TEST_LOCK(lck, gtid);
 #if USE_ITT_BUILD
   if (rc) {
     __kmp_itt_lock_acquired(lck);
   } else {
     __kmp_itt_lock_cancelled(lck);
   }
 #endif /* USE_ITT_BUILD */
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   if (rc && ompt_enabled.ompt_callback_mutex_acquired) {
     ompt_callbacks.ompt_callback(ompt_callback_mutex_acquired)(
-        ompt_mutex_lock, (ompt_wait_id_t)(uintptr_t)lck, codeptr);
+        ompt_mutex_test_lock, (ompt_wait_id_t)(uintptr_t)lck, codeptr);
   }
 #endif
 
   return (rc ? FTN_TRUE : FTN_FALSE);
 
   /* Can't use serial interval since not block structured */
 
 #endif // KMP_USE_DYNAMIC_LOCK
 }
 
 /* try to acquire the lock */
 int __kmpc_test_nest_lock(ident_t *loc, kmp_int32 gtid, void **user_lock) {
 #if KMP_USE_DYNAMIC_LOCK
   int rc;
 #if USE_ITT_BUILD
   __kmp_itt_lock_acquiring((kmp_user_lock_p)user_lock);
 #endif
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   // This is the case, if called from omp_init_lock_with_hint:
   void *codeptr = OMPT_LOAD_RETURN_ADDRESS(gtid);
   if (!codeptr)
     codeptr = OMPT_GET_RETURN_ADDRESS(0);
   if (ompt_enabled.ompt_callback_mutex_acquire) {
     ompt_callbacks.ompt_callback(ompt_callback_mutex_acquire)(
-        ompt_mutex_nest_lock, omp_lock_hint_none,
+        ompt_mutex_test_nest_lock, omp_lock_hint_none,
         __ompt_get_mutex_impl_type(user_lock),
         (ompt_wait_id_t)(uintptr_t)user_lock, codeptr);
   }
 #endif
   rc = KMP_D_LOCK_FUNC(user_lock, test)((kmp_dyna_lock_t *)user_lock, gtid);
 #if USE_ITT_BUILD
   if (rc) {
     __kmp_itt_lock_acquired((kmp_user_lock_p)user_lock);
   } else {
     __kmp_itt_lock_cancelled((kmp_user_lock_p)user_lock);
   }
 #endif
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   if (ompt_enabled.enabled && rc) {
     if (rc == 1) {
       if (ompt_enabled.ompt_callback_mutex_acquired) {
         // lock_first
         ompt_callbacks.ompt_callback(ompt_callback_mutex_acquired)(
-            ompt_mutex_nest_lock, (ompt_wait_id_t)(uintptr_t)user_lock,
+            ompt_mutex_test_nest_lock, (ompt_wait_id_t)(uintptr_t)user_lock,
             codeptr);
       }
     } else {
       if (ompt_enabled.ompt_callback_nest_lock) {
         // lock_next
         ompt_callbacks.ompt_callback(ompt_callback_nest_lock)(
             ompt_scope_begin, (ompt_wait_id_t)(uintptr_t)user_lock, codeptr);
       }

 #if OMPT_SUPPORT && OMPT_OPTIONAL
   // This is the case, if called from omp_init_lock_with_hint:
   void *codeptr = OMPT_LOAD_RETURN_ADDRESS(gtid);
   if (!codeptr)
     codeptr = OMPT_GET_RETURN_ADDRESS(0);
   if (ompt_enabled.enabled) &&
         ompt_enabled.ompt_callback_mutex_acquire) {
       ompt_callbacks.ompt_callback(ompt_callback_mutex_acquire)(
-          ompt_mutex_nest_lock, omp_lock_hint_none,
+          ompt_mutex_test_nest_lock, omp_lock_hint_none,
           __ompt_get_mutex_impl_type(), (ompt_wait_id_t)(uintptr_t)lck,
           codeptr);
     }
 #endif
 
   rc = TEST_NESTED_LOCK(lck, gtid);
 #if USE_ITT_BUILD
   if (rc) {
     __kmp_itt_lock_acquired(lck);
   } else {
     __kmp_itt_lock_cancelled(lck);
   }
 #endif /* USE_ITT_BUILD */
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   if (ompt_enabled.enabled && rc) {
     if (rc == 1) {
       if (ompt_enabled.ompt_callback_mutex_acquired) {
         // lock_first
         ompt_callbacks.ompt_callback(ompt_callback_mutex_acquired)(
-            ompt_mutex_nest_lock, (ompt_wait_id_t)(uintptr_t)lck, codeptr);
+            ompt_mutex_test_nest_lock, (ompt_wait_id_t)(uintptr_t)lck, codeptr);
       }
     } else {
       if (ompt_enabled.ompt_callback_nest_lock) {
         // lock_next
         ompt_callbacks.ompt_callback(ompt_callback_nest_lock)(
             ompt_mutex_scope_begin, (ompt_wait_id_t)(uintptr_t)lck, codeptr);
       }
     }