[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

[Tony Tye via Phabricator via cfe-commits]

t-tye added inline comments.


================
Comment at: include/clang/Basic/Builtins.def:717
+ATOMIC_BUILTIN(__opencl_atomic_fetch_max, "v.", "t")
+
 #undef ATOMIC_BUILTIN
----------------
Will the OpenCL 2.0 memory fences also be supported which also have a memory order and memory scope?


================
Comment at: include/clang/Basic/SyncScope.h:25-29
+  SingleThread      = 0,
+  WorkGroup         = 1,
+  Device            = 2,
+  System            = 3,
+  SubGroup          = 4,
----------------
Since the builtins are being named as __opencl then should these also be named as opencl_ since they are the memory scopes for OpenCL using the OpenCL numeric values?

If another language wants to use memory scopes, would it then add its own langx_* names in a similar way that is done for address spaces where the LangAS enumeration type has values for each distinct language. Each target is then responsible for mapping each language scope to the appropriate target specific scope as is done for address spaces?

If so then the builtins are really supporting the concept of memory scopes and are not language specific as this enumeration can support multiple languages in the same way as the LangAS enumeration supports multiple languages. If so the builtins would be better named to reflect this as @b-sumner suggested.


https://reviews.llvm.org/D28691