[PATCH] D79279: Add overloaded versions of builtin mem* functions

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D79279#2201136 <https://reviews.llvm.org/D79279#2201136>, @rjmccall wrote:

> In D79279#2200916 <https://reviews.llvm.org/D79279#2200916>, @rsmith wrote:
>
>> In D79279#2197176 <https://reviews.llvm.org/D79279#2197176>, @rjmccall wrote:
>>
>>> I thought part of the point of `__builtin_memcpy` was so that C library headers could do `#define memcpy(x, y, z) __builtin_memcpy(x, y, z)`.  If so, the conformance issue touches `__builtin_memcpy` as well, not just calls to the library builtin.

For what it's worth, giving `__builtin_memcpy` broader semantics than `memcpy` wouldn't be unprecedented: it's already the case that `__builtin_memcpy` is usable in constant expressions where plain `memcpy` is not (but there's no macro risk in that case at least since C++ `memcpy` can't be a macro, and a call to memcpy is never an ICE in C).

>> They would have to declare it as well (because C code can `#undef memcpy` and expect to then be able to call a real function), so the `#define` would be pointless.
>
> It wouldn't be pointless; it would enable optimization of direct calls to memcpy (the 99% case) without the compiler having to special-case a function by name.

I mean, yes, in an environment where the compiler didn't special-case the function by name anyway it wouldn't be pointless. I'm not aware of any such environment in popular usage, but that could just be ignorance on my part.

> If we just want memcpy to do the right thing when called directly, that's not ridiculous.  I don't think it would actually have any conformance problems: a volatile memcpy is just a less optimizable memcpy, and to the extent that an address-space-aware memcpy is different from the standard definition, it's probably UB to use the standard definition to copy memory in non-generic address spaces.

I think this is a constraint violation in C; C11 6.5.2.2/2 (a "Constraints" paragraph) requires that "Each argument shall have a type such that its value may be assigned to an object with the unqualified version of the type of its corresponding parameter." So this would be an extension, not just defining UB, and seems like the kind of extension we'd normally diagnose under `-pedantic-errors`, but we could make an exception. I also think it'd be mildly surprising for an explicitly-declared function to allow different argument conversions depending on whether its name is `memcpy`.

It seems to me that you're not entirely comfortable with making `memcpy` and `__builtin_memcpy` differ in this way, and I'm not entirely comfortable with making `memcpy`'s behavior differ from its declared type. Meanwhile, @tstellar's patch wants `__builtin_memcpy` to be overloaded on address space. I don't see a way to address all three concerns at once.

I think it would be reasonable in general to guarantee that our `__builtin_` functions have contracts at least as wide as the underlying C function, but allow them to have extensions, and to keep the plain C functions unextended. I had actually thought we already did that in more cases than we do (but perhaps I was thinking about the LLVM math intrinsics that guarantee to not set `errno`). That would mean that a C standard library implementation is still free to `#define foo(x,y,z) __builtin_foo(x,y,z)`, but if they do, they may pick up extensions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279