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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
