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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 23 11:25:58 PDT 2020


rjmccall added a comment.

In D79279#2170187 <https://reviews.llvm.org/D79279#2170187>, @jfb wrote:

> In D79279#2170157 <https://reviews.llvm.org/D79279#2170157>, @rjmccall wrote:
>
> > I think the argument is treated as if it were 1 if not given.  That's all that ordinary memcpy formally guarantees, which seems to work fine (semantically, if not performance-wise) for pretty much everything today.
>
>
> I'm not sure that's true: consider a `memcpy` implementation which copies some bytes twice (at different access size, there's an overlap because somehow it's more efficient). That would probably violate the programmer's expectations, and I don't think `volatile` nor atomic `memcpy` allow this (but regular `memcpy` does).


Yes, that's true, if you need an only-accessed-once guarantee, that's above and beyond just a minimum access size.  I agree that `volatile` would need to make this guarantee.

>> Do you think it'd be useful to have different guarantees for different operands?  I guess it could come up, but it'd be a whole lot of extra complexity that I can't imagine we'd ever support.
> 
> You mean, if `element_size` is passed then you get different guarantees?

No, sorry, I mean different guarantees for the different pointer operands.  In principle, we could allow you to say that the memcpy has to be done with 4-byte accesses from the source and 2-byte accesses to the destination.  That's implementable but a lot of work.

>> If one of the arguments is `volatile`, arguably the minimum access width (if given) needs to be exact.  If we don't support that right now, it's okay to make it an error, which is basically you've already done with the `_Atomic volatile` diagnostic.
> 
> Agreed. `volatile` with size makes a lot of sense, and the IR version of it, once created, ought to not be able to widen accesses. `volatile` without a size specified makes sense too, because you just want a single read and a single write, don't care about tearing.

Right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279





More information about the cfe-commits mailing list