<div dir="ltr"><div dir="ltr"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 11, 2018 at 6:50 PM James Y Knight <<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>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.<br></div><div><br></div><div>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.</div></div></div></div></div></div></div></div></div></div></div></div></div></div></blockquote><div> </div><div><div>So, anyhow, assuming this change does stay in the standard, how should it be implemented?</div><div><br></div><div>The important prerequisite question I think is:</div><div>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?)</div><div><br></div><div>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...</div><div><br></div><div>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:</div><div>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.<br></div><div>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.<br></div><div><br></div><div><br></div><div>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.).<br></div><div><br></div><div><div>bool atomic<T>::compare_exchange_strong(T& expected, T desired, std::memory_order success, std::memory_order failure) {</div><div>  if (!__builtin_object_contains_padding(T)) // new builtin function</div><div>    return compare_exchange_strong_internal(expected, desired, success, failure);</div><div><br></div><div>  // Can't write to "expected" except on failure, so we need a temporary.</div><div>  alignas(T) char tmp_expected[sizeof(tmp_expected)];</div><div><br></div><div>  // Need an atomic load which takes a pointer to the destination memory.</div><div>  // GCC has this builtin, but clang may need to add a `__c11_atomic_load_into` for libc++.</div><div>  __atomic_load(&_val, (T*)tmp_expected, success);</div><div>  __builtin_copy_nonpadding((T*)tmp_expected, &expected); // new builtin function<br></div><div><br></div><div>  if (compare_exchange_strong_internal(tmp_expected, desired, success, failure))</div><div>    return true;</div><div>  memcpy(expected, tmp_expected);</div><div>  return false;</div><div>}</div></div><div><br></div><div>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.<br></div></div><div><br></div><div><br></div><div>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.</div></div></div></div></div>