[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins

[Tony Tye via Phabricator via cfe-commits]

t-tye added inline comments.


================
Comment at: include/clang/Basic/SyncScope.h:46
+  Scopes.push_back(SyncScope::OpenCLSubGroup);
+  return Scopes;
+}
----------------
Should there be an assert/static_assert in case SyncScope enum grows due to other languages?


================
Comment at: include/clang/Basic/SyncScope.h:59
+    return "opencl_subgroup";
+  }
+}
----------------
Should there be a default/assert/static_assert to allow SyncScope enum to grow due to other languages?


================
Comment at: lib/CodeGen/CGAtomic.cpp:696
+    if (S != Default)
+      SI->addCase(Builder.getInt32(static_cast<unsigned>(S)), B);
+
----------------
Is it documented in the SyncScope enum that the enumerator values are in fact the values used for source language runtime values? Seems if other languages want to use scopes they may may have a different ordering. That would imply that there would be a function to map a SyncScope value to the value used by the source language. For OpenCL the mapping is identity.

The memory ordering has the isValidAtomicOrderingCABI() that does a similar thing.


https://reviews.llvm.org/D36580