From 1f26e1d8ba372f4c638f9c0cdae7566e349b9b9a Mon Sep 17 00:00:00 2001
From: Jeremy Kemp <jeremy@jeremykemp.co.uk>
Date: Tue, 7 Sep 2021 12:47:44 +0100
Subject: [PATCH] Fix memory model issue in `atomic_flag`. (#1283)

* Fix memory model issue in atomic_flag.

In atomic_flag sub-tests that modify local memory, compilers may re-order memory accesses between the local and global address spaces which can lead to incorrect test failures.

This commit ensures that both local and global memory operations are fenced to prevent this re-ordering from occurring.

Fixes #134.

* Clang format changes.

* Added missing global acquire which is necessary for the corresponding global release.

Thanks to @jlewis-austin for spotting.

* Clang format changes.

* Match the condition for applying acquire/release fences.
---
 test_conformance/c11_atomics/test_atomics.cpp | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/test_conformance/c11_atomics/test_atomics.cpp b/test_conformance/c11_atomics/test_atomics.cpp
index c3a190b7..38b4e9a7 100644
--- a/test_conformance/c11_atomics/test_atomics.cpp
+++ b/test_conformance/c11_atomics/test_atomics.cpp
@@ -1657,12 +1657,18 @@ public:
       "  for(cnt = 0; !stop && cnt < threadCount; cnt++) // each thread must find critical section where it is the first visitor\n"
       "  {\n"
       "    bool set = atomic_flag_test_and_set" + postfix + "(&destMemory[cnt]" + memoryOrderScope + ");\n";
-    if (MemoryOrder() == MEMORY_ORDER_RELAXED || MemoryOrder() == MEMORY_ORDER_RELEASE)
-      program += "    atomic_work_item_fence(" +
-                 std::string(LocalMemory() ? "CLK_LOCAL_MEM_FENCE, " : "CLK_GLOBAL_MEM_FENCE, ") +
-                 "memory_order_acquire," +
-                 std::string(LocalMemory() ? "memory_scope_work_group" : (UseSVM() ? "memory_scope_all_svm_devices" : "memory_scope_device") ) +
-                 ");\n";
+    if (MemoryOrder() == MEMORY_ORDER_RELAXED
+        || MemoryOrder() == MEMORY_ORDER_RELEASE || LocalMemory())
+        program += "    atomic_work_item_fence("
+            + std::string(LocalMemory()
+                              ? "CLK_LOCAL_MEM_FENCE | CLK_GLOBAL_MEM_FENCE, "
+                              : "CLK_GLOBAL_MEM_FENCE, ")
+            + "memory_order_acquire,"
+            + std::string(LocalMemory()
+                              ? "memory_scope_work_group"
+                              : (UseSVM() ? "memory_scope_all_svm_devices"
+                                          : "memory_scope_device"))
+            + ");\n";
 
     program +=
       "    if (!set)\n"
@@ -1683,12 +1689,18 @@ public:
       "        stop = 1;\n"
       "      }\n";
 
-    if (MemoryOrder() == MEMORY_ORDER_ACQUIRE || MemoryOrder() == MEMORY_ORDER_RELAXED)
-      program += "      atomic_work_item_fence(" +
-                 std::string(LocalMemory() ? "CLK_LOCAL_MEM_FENCE, " : "CLK_GLOBAL_MEM_FENCE, ") +
-                 "memory_order_release," +
-                 std::string(LocalMemory() ? "memory_scope_work_group" : (UseSVM() ? "memory_scope_all_svm_devices" : "memory_scope_device") ) +
-                 ");\n";
+    if (MemoryOrder() == MEMORY_ORDER_ACQUIRE
+        || MemoryOrder() == MEMORY_ORDER_RELAXED || LocalMemory())
+        program += "      atomic_work_item_fence("
+            + std::string(LocalMemory()
+                              ? "CLK_LOCAL_MEM_FENCE | CLK_GLOBAL_MEM_FENCE, "
+                              : "CLK_GLOBAL_MEM_FENCE, ")
+            + "memory_order_release,"
+            + std::string(LocalMemory()
+                              ? "memory_scope_work_group"
+                              : (UseSVM() ? "memory_scope_all_svm_devices"
+                                          : "memory_scope_device"))
+            + ");\n";
 
     program +=
       "      atomic_flag_clear" + postfix + "(&destMemory[cnt]" + MemoryOrderScopeStrForClear() + ");\n"