[Openmp-commits] [openmp] 95cefac - [OpenMP] Fix crashing critical section with hint clause

[Hansang Bae via Openmp-commits]

Author: Hansang Bae
Date: 2021-05-24T17:25:01-05:00
New Revision: 95cefacfe1c1fe920f22b749f17f630925bd6094

URL: https://github.com/llvm/llvm-project/commit/95cefacfe1c1fe920f22b749f17f630925bd6094
DIFF: https://github.com/llvm/llvm-project/commit/95cefacfe1c1fe920f22b749f17f630925bd6094.diff

LOG: [OpenMP] Fix crashing critical section with hint clause

Runtime was using the default lock type without using the hint.

Differential Revision: https://reviews.llvm.org/D102955

Added: 
    openmp/runtime/test/critical/omp_critical_with_hint.c

Modified: 
    openmp/runtime/src/kmp_csupport.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_csupport.cpp b/openmp/runtime/src/kmp_csupport.cpp
index 59d0dec50534..802a40442dad 100644
--- a/openmp/runtime/src/kmp_csupport.cpp
+++ b/openmp/runtime/src/kmp_csupport.cpp
@@ -1494,13 +1494,13 @@ void __kmpc_critical_with_hint(ident_t *loc, kmp_int32 global_tid,
   kmp_dyna_lock_t *lk = (kmp_dyna_lock_t *)crit;
   // Check if it is initialized.
   KMP_PUSH_PARTITIONED_TIMER(OMP_critical_wait);
+  kmp_dyna_lockseq_t lockseq = __kmp_map_hint_to_lock(hint);
   if (*lk == 0) {
-    kmp_dyna_lockseq_t lckseq = __kmp_map_hint_to_lock(hint);
-    if (KMP_IS_D_LOCK(lckseq)) {
+    if (KMP_IS_D_LOCK(lockseq)) {
       KMP_COMPARE_AND_STORE_ACQ32((volatile kmp_int32 *)crit, 0,
-                                  KMP_GET_D_TAG(lckseq));
+                                  KMP_GET_D_TAG(lockseq));
     } else {
-      __kmp_init_indirect_csptr(crit, loc, global_tid, KMP_GET_I_TAG(lckseq));
+      __kmp_init_indirect_csptr(crit, loc, global_tid, KMP_GET_I_TAG(lockseq));
     }
   }
   // Branch for accessing the actual lock object and set operation. This
@@ -1533,11 +1533,11 @@ void __kmpc_critical_with_hint(ident_t *loc, kmp_int32 global_tid,
     }
 #endif
 #if KMP_USE_INLINED_TAS
-    if (__kmp_user_lock_seq == lockseq_tas && !__kmp_env_consistency_check) {
+    if (lockseq == lockseq_tas && !__kmp_env_consistency_check) {
       KMP_ACQUIRE_TAS_LOCK(lck, global_tid);
     } else
 #elif KMP_USE_INLINED_FUTEX
-    if (__kmp_user_lock_seq == lockseq_futex && !__kmp_env_consistency_check) {
+    if (lockseq == lockseq_futex && !__kmp_env_consistency_check) {
       KMP_ACQUIRE_FUTEX_LOCK(lck, global_tid);
     } else
 #endif
@@ -1614,7 +1614,8 @@ void __kmpc_end_critical(ident_t *loc, kmp_int32 global_tid,
   KC_TRACE(10, ("__kmpc_end_critical: called T#%d\n", global_tid));
 
 #if KMP_USE_DYNAMIC_LOCK
-  if (KMP_IS_D_LOCK(__kmp_user_lock_seq)) {
+  int locktag = KMP_EXTRACT_D_TAG(crit);
+  if (locktag) {
     lck = (kmp_user_lock_p)crit;
     KMP_ASSERT(lck != NULL);
     if (__kmp_env_consistency_check) {
@@ -1624,11 +1625,11 @@ void __kmpc_end_critical(ident_t *loc, kmp_int32 global_tid,
     __kmp_itt_critical_releasing(lck);
 #endif
 #if KMP_USE_INLINED_TAS
-    if (__kmp_user_lock_seq == lockseq_tas && !__kmp_env_consistency_check) {
+    if (locktag == locktag_tas && !__kmp_env_consistency_check) {
       KMP_RELEASE_TAS_LOCK(lck, global_tid);
     } else
 #elif KMP_USE_INLINED_FUTEX
-    if (__kmp_user_lock_seq == lockseq_futex && !__kmp_env_consistency_check) {
+    if (locktag == locktag_futex && !__kmp_env_consistency_check) {
       KMP_RELEASE_FUTEX_LOCK(lck, global_tid);
     } else
 #endif

diff  --git a/openmp/runtime/test/critical/omp_critical_with_hint.c b/openmp/runtime/test/critical/omp_critical_with_hint.c
new file mode 100644
index 000000000000..68c726917ae1
--- /dev/null
+++ b/openmp/runtime/test/critical/omp_critical_with_hint.c
@@ -0,0 +1,53 @@
+// RUN: %libomp-compile-and-run
+#include <stdio.h>
+#include <omp.h>
+#include "omp_testsuite.h"
+
+int test_omp_critical(int iter) {
+  int sum;
+  int known_sum;
+
+  sum = 0;
+#pragma omp parallel
+  {
+    int mysum = 0;
+    int i;
+#pragma omp for
+    for (i = 0; i < 1000; i++)
+      mysum = mysum + i;
+
+    switch (iter % 4) {
+    case 0:
+#pragma omp critical(c0) hint(omp_sync_hint_uncontended)
+      sum = mysum + sum;
+      break;
+    case 1:
+#pragma omp critical(c1) hint(omp_sync_hint_contended)
+      sum = mysum + sum;
+      break;
+    case 2:
+#pragma omp critical(c2) hint(omp_sync_hint_nonspeculative)
+      sum = mysum + sum;
+      break;
+    case 3:
+#pragma omp critical(c3) hint(omp_sync_hint_speculative)
+      sum = mysum + sum;
+      break;
+    default:;
+    }
+  }
+  known_sum = 999 * 1000 / 2;
+  return (known_sum == sum);
+}
+
+int main() {
+  int i;
+  int num_failed = 0;
+
+  for (i = 0; i < 4 * REPETITIONS; i++) {
+    if (!test_omp_critical(i)) {
+      num_failed++;
+    }
+  }
+  return num_failed;
+}