rsmith added inline comments.
================ Comment at: clang/docs/LanguageExtensions.rst:2435-2437 +* ``__builtin_memcpy_overloaded(QUAL void *dst, QUAL const void *src, size_t byte_size, size_t byte_element_size = <unspecified>)`` +* ``__builtin_memmove_overloaded(QUAL void *dst, QUAL const void *src, size_t byte_size, size_t byte_element_size = <unspecified>)`` +* ``__builtin_memset_overloaded(QUAL void *dst, unsigned char val, size_t byte_size, size_t byte_element_size = <unspecified>)`` ---------------- What happens if `byte_element_size` does not divide `byte_size`? ================ Comment at: clang/docs/LanguageExtensions.rst:2435-2437 +* ``__builtin_memcpy_overloaded(QUAL void *dst, QUAL const void *src, size_t byte_size, size_t byte_element_size = <unspecified>)`` +* ``__builtin_memmove_overloaded(QUAL void *dst, QUAL const void *src, size_t byte_size, size_t byte_element_size = <unspecified>)`` +* ``__builtin_memset_overloaded(QUAL void *dst, unsigned char val, size_t byte_size, size_t byte_element_size = <unspecified>)`` ---------------- rsmith wrote: > What happens if `byte_element_size` does not divide `byte_size`? Did you really mean `void*` here? I've been pretty confused by some of the stuff happening below that seems to depend on the actual type of the passed pointer, which would make more sense if you meant `QUAL T *` here rather than `QUAL void*`. Do the builtins do different things for different argument pointee types or not? ================ Comment at: clang/docs/LanguageExtensions.rst:2444-2445 +parameter can be provided to specify element access size in bytes. When the +element size is provided, the memory will be accessed with a sequence of +operations of size equal to or a multiple of the pointer's element size. The +order of operations is unspecified, and each access has unordered atomic ---------------- "the pointer's element size" -- do you mean "the provided element size"? Does the element size need to be a compile-time constant? (Presumably, but you don't say so.) ================ Comment at: clang/docs/LanguageExtensions.rst:2446-2447 +operations of size equal to or a multiple of the pointer's element size. The +order of operations is unspecified, and each access has unordered atomic +semantics. This means that reads and writes do not tear at the individual +element level, and they each occur exactly once, but the order in which they ---------------- Presumably this means that it's an error if we don't provide lock-free atomic access for that size. Would be worth saying so. ================ Comment at: clang/docs/LanguageExtensions.rst:2450-2451 +occur (and in which they are observable) can only be guaranteed using +appropriate fences around the function call. Atomic memory operations must be to +memory locations which are aligned to no less than the element size. + ---------------- What happens if they're not? Is it UB, or is it just not guaranteed to be atomic? ================ Comment at: clang/docs/LanguageExtensions.rst:2456 + +These can be used as building blocks for different facitilies: + ---------------- *facilities ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8937-8938 + "(%0 is volatile)">; +def err_elsz_on_sizeless : Error< + "element size must not be on a sizeless type (%0 invalid)">; +def err_elsz_must_be_lock_free : Error< ---------------- Given the new documentation, I would expect you don't need this any more. ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8941-8943 +def err_atomic_builtin_ext_size_mismatches_el : Error< + "number of bytes to copy must be a multiple of pointer element size, " + "got %0 bytes to copy with element size %1 for %2">; ---------------- Presumably the number of bytes need not be a compile-time constant? It's a bit weird to produce an error rather than a warning on a case that would be valid but (perhaps?) UB if the argument were non-constant. ================ Comment at: clang/lib/AST/ExprConstant.cpp:8793 + BuiltinOp == Builtin::BI__builtin_memmove_overloaded || BuiltinOp == Builtin::BI__builtin_wmemmove; ---------------- jfb wrote: > If we end up making alignment a runtime constraint, then I'll need to check > it in consteval. Otherwise I don't think we need to check anything since Sema > ought to have done all the required checks already. I don't see how you can check the alignment at compile time given a `void*` argument. We presumably need to check that the element size (if given) divides the total size, assuming the outcome is UB if not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79279/new/ https://reviews.llvm.org/D79279 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits