[PATCH] D79279: Allow volatile parameters to __builtin_mem{cpy,move,set}

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat May 2 19:04:55 PDT 2020


rjmccall added a comment.

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

> In D79279#2015983 <https://reviews.llvm.org/D79279#2015983>, @rjmccall wrote:
>
> > Most of the complexity of this patch is introduced by the decision to type-check these calls with a volatile-typed parameter, which seems like it does nothing but cause problems.  If your goal is to make these functions do the right thing when given arbitrary pointer types, I think you need to give these calls special type-checking semantics.  Done right, that will also let you e.g. accept pointers into arbitrary address spaces.  But I'm not sure how good of an idea this actually is at base, since these builtins are typically used for direct calls to their associated library functions.
>
>
> You mean: in `Builtins,def` allow a `?` modifier on `volatile` (so, `D?`) to denote overloading on `volatile`, and consume that overload directly while type checking? That seems fine to me. Just want to make sure that's what you have in mind.


I don't know how that `?` would actually work, but more importantly, no, that's not really what I meant.  The current semantics of these builtins are that they are not polymorphic.  I think the right way to understand what you're trying to do is that you're trying to make them polymorphic over qualifiers.  That means that we can do a `__builtin_memset` of a `volatile void *`, yes, but it also means that we can do a `__builtin_memset` of a `volatile restrict __local void *`.  (Now, some qualifiers don't make sense to honor because they'd be a radical change in behavior — we wouldn't want to honor `__strong` by doing a bunch of ARC assignments, for example.  But most qualifiers do still make sense for these operations.)  And in fact there's a certain amount of precedent for this because we already honor the presumed alignment of the pointer before it's converted to `void*`.

Unfortunately, the only way to do this kind of polymorphism in Clang today is to give the entire builtin custom type-checking with `t`.  And you'll need to do a bunch of work to make sure that the behavior of this builtin is consistent with an ordinary call to `memcpy` when e.g. the argument is a class type with overloaded conversion operators to pointer types.  But I think that's what you're signing up for.

I do think this is a somewhat debatable change in the behavior of these builtins, though.


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