[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D87974#2439043 <https://reviews.llvm.org/D87974#2439043>, @jwakely wrote:

> In D87974#2438723 <https://reviews.llvm.org/D87974#2438723>, @zoecarver wrote:
>
>> In D87974#2438682 <https://reviews.llvm.org/D87974#2438682>, @BillyONeal wrote:
>>
>>>> Are they actually the same, with the same handling of corner cases like unions and tail padding?
>>>> There's more to this than just the name, and if they aren't the same, it seems better to have two names.
>>>
>>> They are both implementing the same C++ feature, with the same desired semantics of zeroing out any bits in the object representation that aren't in the value representation. If they differ, one or the other would have a bug.
>
> Do they support non-trivially copyable types? That isn't required for the atomic compare exchange feature, but is relevant for a feature exposed to users. What about extensions like zero-sized arrays or C99 flexible array members?

As far as MSVC is concerned this isn't "exposed to users".

> What about extensions like zero-sized arrays or C99 flexible array members?

We don't have those extensions at all so it's irrelevant to talk about what the builtin would do with them in our case. (And at such time we would add such extensions presumably we would match gcc's behavior since again, there's no reason for the behavior to differ here)

>> I agree, they either need to be identical (including corner cases) or there needs to be two of them (i.e., GCC ships both `__builtin_zero_non_value_bits` and `__builtin_clear_padding` and the first is the same as MSVC, Clang, and NVCC).
>
> GCC doesn't need to support both. It only works with libstdc++ so it only needs to support the one used by libstdc++ (although there is a patch to add `-stdlib=libc++` to GCC).
>
> If libstdc++ uses `__has_builtin` to check what the compiler supports then Clang doesn't even need to support GCC's built-in, because libstdc++ wouldn't use it if not supported (and could use `__builtin_zero_non_value_bits` instead when supported).
>
> The Intel compiler would need to support both though.
>
>>>> Is there a specification for __builtin_zero_non_value_bits available somewhere?
>>>
>>> I don't know if there is a formal spec for it beyond the actual C++ standard.
>>
>> I think P0528 is the relevant paper but other than that, no, there's not a spec. I think that's going to be the most time sensitive part of implementing this: coming up with the spec and making sure all the tests pass on all the implementations.
>
> GCC has publicly available documentation describing its built-in, and publicly available tests for it. That's the kind of spec I'm looking for.

We don't consider it "publicly available" so there isn't going to be that kind of documentation for it. I don't see a serious problem with the gcc version of that builtin supporting a superset of the functionality of the equivalent msvc builtin.

Of course if it's already publicly documented for you the horse has presumably already left the barn which makes the discussion moot?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974