Fix omp_sections_nowait.c test to address Bugzilla Bug 28336

[jlpeyton]

Summary

This rewrite of the omp_sections_nowait.c test file causes it to hang if the nowait is not respected. If the nowait isn't respected, the lone thread which can escape the first sections construct will just sleep at a barrier which shouldn't exist. All reliance on timers is taken out. For good measure, the test makes sure that all eight sections are executed as well. The test should take no longer than a few seconds on any modern machine.

[hfinkel]

Thanks! LGTM.

test/worksharing/sections/omp_sections_nowait.c

23 ↗

(On Diff #62234)

I don't recall how this part of OpenMP's memory model is specified. Do you technically need an omp flush(release) here too to force this thread to synchronize its local memory view with the global one?

[AndreyChurbanov]

test/worksharing/sections/omp_sections_nowait.c

23 ↗

(On Diff #62234)

In theory all accesses to "release" variable are data races here. And flush won't change this. So theoretically atomic reads/writes could help the test be absolutely correct.

But in practice these data races are benign, so I think it is OK to rely on compiler to honor volatile keyword and the fact that int type has all reads and writes atomic (unless you artificially align it on odd boundary).

So I would not introduce complication with omp atomic read/write for variable "release" now.

Just my 2 cents.

[hfinkel]

test/worksharing/sections/omp_sections_nowait.c

23 ↗

(On Diff #62234)

Fair enough; fine by me. I'm fine with our test suite testing non-standard guarantees we expect our implementation (and any reasonable implementation) to provide.

openmp/trunk/runtime/test/worksharing/sections/omp_sections_nowait.c

 // RUN: %libomp-compile-and-run
 #include <stdio.h>
 #include "omp_testsuite.h"
-#include "omp_my_sleep.h"
 
-int test_omp_sections_nowait()
+/*
+ * This test will hang if the nowait is not working properly
+ *
+ * It relies on a thread skipping to the second sections construct to
+ * release the threads in the first sections construct
+ *
+ * Also, since scheduling of sections is implementation defined, it is
+ * necessary to have all four sections in the second sections construct
+ * release the threads since we can't guarantee which section a single thread
+ * will execute.
+ */
+volatile int release;
+volatile int count;
+
+void wait_for_release_then_increment(int rank)
 {
-  int result;
-  int count;
-  int j;
+  fprintf(stderr, "Thread nr %d enters first section"
+    " and waits.\n", rank);
+  while (release == 0);
+  #pragma omp atomic
+  count++;
+}
 
-  result = 0;
+void release_and_increment(int rank)
+{
+  fprintf(stderr, "Thread nr %d sets release to 1\n", rank);
+  release = 1;
+  #pragma omp flush(release)
+  #pragma omp atomic
+  count++;
+}
+
+int test_omp_sections_nowait()
+{
+  release = 0;
   count = 0;
 
-  #pragma omp parallel
+  #pragma omp parallel num_threads(4)
   {
     int rank;
     rank = omp_get_thread_num ();
     #pragma omp sections nowait
     {
       #pragma omp section
       {
-        fprintf(stderr, "Thread nr %d enters first section"
-          " and gets sleeping.\n", rank);
-        my_sleep(SLEEPTIME);
-        count = 1;
-        fprintf(stderr, "Thread nr %d woke up an set"
-          " count to 1.\n", rank);
-        #pragma omp flush(count)
+        wait_for_release_then_increment(rank);
+      }
+      #pragma omp section
+      {
+        wait_for_release_then_increment(rank);
+      }
+      #pragma omp section
+      {
+        wait_for_release_then_increment(rank);
       }
       #pragma omp section
       {
-        fprintf(stderr, "Thread nr %d executed work in the"
-          " first section.\n", rank);
+        fprintf(stderr, "Thread nr %d enters first sections and goes "
+          "immediately to next sections construct to release.\n", rank);
+        #pragma omp atomic
+        count++;
       }
     }
     /* Begin of second sections environment */
     #pragma omp sections
     {
       #pragma omp section
       {
-        fprintf(stderr, "Thread nr %d executed work in the"
-          " second section.\n", rank);
+        release_and_increment(rank);
+      }
+      #pragma omp section
+      {
+        release_and_increment(rank);
+      }
+      #pragma omp section
+      {
+        release_and_increment(rank);
       }
       #pragma omp section
       {
-        fprintf(stderr, "Thread nr %d executed work in the"
-          " second section and controls the value of count\n", rank);
-        if (count == 0)
-          result = 1;
-        fprintf(stderr, "count was %d\n", count);
+        release_and_increment(rank);
       }
     }
   }
-  return result;
+  // Check to make sure all eight sections were executed
+  return (count==8);
 }
 
 int main()
 {
   int i;
   int num_failed=0;
 
   for(i = 0; i < REPETITIONS; i++) {
     if(!test_omp_sections_nowait()) {
       num_failed++;
     }
   }
   return num_failed;
 }