t-tye accepted this revision. t-tye added a comment. This revision is now accepted and ready to land.
LGTM other than suggested documentation and static_assert/unreachable comments. ================ Comment at: lib/CodeGen/CGAtomic.cpp:696 + if (S != Default) + SI->addCase(Builder.getInt32(static_cast<unsigned>(S)), B); + ---------------- rjmccall wrote: > t-tye wrote: > > 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. > The values in the SyncScope enum are the source language values. We already > have a step to translate them into LLVM values when we generate a native LLVM > construct. To the extent that we call into a runtime instead, none of that > code has been written to be runtime-agnostic at all, and I've been assuming > that we're basically okay with that, at least for now. That sounds reasonable to me. When/if another language comes along the task of mapping the multiple ABIs can be done then. Still wanted to make sure it is clear that the enum values in the SyncScope enum are documented as BEING the OpenCL runtime ABI values and not some arbitrary list as in other enums such as the address space enum. https://reviews.llvm.org/D36580 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
