[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

Sameer Sahasrabuddhe via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 10 19:42:08 PDT 2020


sameerds added a comment.

The commit summary needs improvement. The syntax is not really necessary there, but instead this needs a better explanation of how this builtin fits in with the overall scheme of language-specific and target-specific details of an atomic operation. For example, is this meant only for OpenCL? Does it work with CUDA? Or HIP? What is the behaviour for scope in C++?



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7860-7863
+def err_memory_fence_has_invalid_memory_order : Error<
+  "memory order argument to fence operation is invalid">;
+def err_memory_fence_has_invalid_synch_scope : Error<
+  "synchronization scope argument to fence operation is invalid">;
----------------
Just above this addition, atomic op seems to emit a warning for invalid memory order. Should that be the case with fence too?


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3707
+    Value *Scope = EmitScalarExpr(E->getArg(1));
+    auto ScopeModel = AtomicScopeModel::create(AtomicScopeModelKind::OpenCL);
+
----------------
The proposed builtin does not claim to be an OpenCL builtin, so it's probably not correct to simply assume the OpenCL model. Should the model be chosen based on the source language specified?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75917/new/

https://reviews.llvm.org/D75917





More information about the cfe-commits mailing list