t-tye added inline comments.
================
Comment at: include/clang/Basic/SyncScope.h:23
+enum class SyncScope {
+ OpenCLWorkItem = 0,
+ OpenCLWorkGroup = 1,
----------------
The OpenCL workitem scope is only used for image fences and does not apply to
atomic operations so not sure that it should be in this enumeration which is
used only for memory atomics.
================
Comment at: lib/CodeGen/TargetInfo.cpp:7554-7555
+ switch (S) {
+ case SyncScope::OpenCLWorkItem:
+ Name = "singlethread";
+ break;
----------------
OpenCL only uses workitem for image fences which are not the same as atomic
memory fences.
For image fences I don't think it would map to singlethread which is intended
for signal handler support to ensure a signal handler has visibility of the
updates done by a thread which is more of an optimization barrier. In contrast
an image fence may need to flush caches to make the image and vector access
paths coherent in a single thread.
Since this patch does not support fences probably want to leave workitem scope
out. Current AMDGPU targets do not need to do anything for an OpenCL image
fence, but other targets may need to generate an intrinsic since there is no
LLVMIR instruction for this.
================
Comment at: lib/Headers/opencl-c.h:13145-13150
memory_scope_work_item,
memory_scope_work_group,
memory_scope_device,
memory_scope_all_svm_devices,
+#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups)
memory_scope_sub_group
----------------
Do these have to have the same values as the SycnScope enumeration? Should that
be ensured in a similar way to the memory_order enumeration?
================
Comment at: lib/Sema/SemaChecking.cpp:3103
+ Diag(Scope->getLocStart(),
+ diag::err_atomic_op_has_non_constant_synch_scope)
+ << Scope->getSourceRange();
----------------
IIRC OpenCL allows the scope to be a runtime value. So will doing this will
likely cause failures in conformance?
https://reviews.llvm.org/D28691
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
