[cfe-dev] Clang builtins for C++20 STL features

[James Y Knight via cfe-dev]

On Tue, Dec 11, 2018 at 6:50 PM James Y Knight <jyknight at google.com> wrote:

> 3. Finally, if the decision is to go forward with P0528 anyways, and if
> padding bits *can* arbitrarily change, then `__builtin_clear_padding(T*
> ptr)` is not a theoretically sound interface, because the padding bits
> could change again, after the call to clear them returns.
>
> We'd need to either implement the support within the atomic builtins
> themselves (which, as noted, cannot be done within the existing GCC
> builtins), or define some alternative interface.
>

So, anyhow, assuming this change does stay in the standard, how should it
be implemented?

The important prerequisite question I think is:
Q: Are the relevant compilers (e.g., GCC, MSVC, Clang) able to guarantee
that it's possible to set padding bytes in an object's memory (e.g. by
writing via casting to char*, or memcpy), and then reliably read it back
unchanged (in the same manner), as long as there have been no intervening
writes to the object? (That is to say, could the previously proposed
__builtin_clear_padding(T*) function actually be properly implemented by
all the relevant C++ compilers?)

I suspect, but am not certain, that the answer is "yes". (And again -- if
that is true in practice, I'd propose that the spec should just specify it
and make everyone's lives easier). Anyhow, continuing with that
assumption...

Using a __builtin_clear_padding function to zero the padding in any new
values before storing them, such that the atomic's padding in memory is
guaranteed to be zeroed seems a reasonable implementation. However there's
two issues:
1. That would be an ABI breakage -- any code compiled with the current
standard library does not zero its padding before storing into std::atomic
objects, so if a new release has a compare_exchange function that requires
padding be zero, it'll fail.
2. std::atomic_ref is tricky. Possibly it would be acceptable for the
atomic_ref constructor to zero out the padding to the underlying object.
I'm not sure if such an extraneous write is allowable or not. At least, it
would need to be done as an atomic compare-exchange operation, since you
may create a new atomic_ref pointing to the same storage as one already
being used concurrently.


The other option is to modify only the compare-exchange operation. We could
do it something like the following. ("compare_exchange_strong_internal"
here is the non-padding-aware existing version of the operation; that is,
effectively either __atomic_compare_exchange or
__c11_atomic_compare_exchange_strong, depending on the stdlib and compiler,
as it is now.).

bool atomic<T>::compare_exchange_strong(T& expected, T desired,
std::memory_order success, std::memory_order failure) {
  if (!__builtin_object_contains_padding(T)) // new builtin function
    return compare_exchange_strong_internal(expected, desired, success,
failure);

  // Can't write to "expected" except on failure, so we need a temporary.
  alignas(T) char tmp_expected[sizeof(tmp_expected)];

  // Need an atomic load which takes a pointer to the destination memory.
  // GCC has this builtin, but clang may need to add a
`__c11_atomic_load_into` for libc++.
  __atomic_load(&_val, (T*)tmp_expected, success);
  __builtin_copy_nonpadding((T*)tmp_expected, &expected); // new builtin
function

  if (compare_exchange_strong_internal(tmp_expected, desired, success,
failure))
    return true;
  memcpy(expected, tmp_expected);
  return false;
}

I'd note that compare_exchange_weak function needs to do the same thing --
while the 'weak' variant is allowed to fail spuriously, it shouldn't
deterministically fail spuriously. So, if called repeatedly with a value of
"expected" which has intentionally-wrong padding, it still ought to
succeed, according to the C++20 spec.


Or -- instead of adding the two or three new builtins noted above, we could
also add a new "atomic_compare_exchange_ignoring_padding" builtin, doing
all of the above internally.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181220/1efacfe5/attachment.html>