RFR: 8186166: Generalize Atomic::cmpxchg with templates

[David Holmes]

Hi Roman,

On 16/08/2017 12:22 AM, Roman Kennke wrote:
> Am 14.08.2017 um 11:56 schrieb Kim Barrett:
>>> On Aug 14, 2017, at 10:28 AM, Roman Kennke <rkennke at redhat.com> wrote:
>>>
>>> I still don't understand why we want to allow for different types for
>>> the new and expected values and the memory location (T, D and U). We
>>> haven't allowed for this before, why are we doing it now?
>> Different types are only permitted in the pointer value case,
>> e.g. cmpxchg(T*, D* volatile*, U*).
>>
>> The compare_value type is permitted to differ from the destination's
>> value type only in cv-qualifiers.  Consider a member variable "var" of
>> type int*.
>>
>>    const int* old_value = var;
>>    ... compute new_value ...
>>    cmpxchg(new_value, &var, old_value);
>>
>> It is perfectly reasonable and appropriate to const-qualify
>> old_value's type in that way.  But that would require a source level
>> cast if cmpxchg required an exact type match for *dest and
>> compare_value.
>>
>> We permit the exchange_value type to be less cv-qualified than the
>> *dest type.  We also permit derived to base conversions.  Both of
>> those are natural and type-safe, and may eliminate the need for some
>> source level casts.
>>
>>> We haven't allowed for this before, why are we doing it now?
>> Because of the complete type-discarding done by cmpxchg_ptr, I suggest
>> we have no clue what our current code might or might not be doing.
>>
> I don't see that anywhere. The only situations that I could find are a
> few places where dest is cast to (volatile foo*), which makes me
> question whether that dest field should be marked volatile in the first
> place. I see no patterns as you described above involving any const

I think it is the very existence of cmpxchg_ptr, to isolate the cases 
involving pointers, that is at issue. There should not be a need for 
cmpxchg_ptr - a well-typed cmpxchg should handle all type variants. And 
that requires the template mechanisms.

> casts. And I am absolutely against introducing some machinery that is
> not actually used. If it's not used, it's dead, and only increases debt
> rather than decrease it. (Note: the situation would be different if it
> were a public API. but it isn't)

I don't see how this will be unused dead code - it will be used whenever 
pointers are used with cmpxchg. ??

Cheers,
David