RFR (M): 8184334: Generalizing Atomic with templates

[David Holmes]

On 8/08/2017 1:23 AM, Andrew Haley wrote:
> On 07/08/17 00:32, Kim Barrett wrote:
>> I'm looking for feedback on this before I try to carry it any further.
> 
> I don't like it because it converts pointers to operand types before
> calling the back end.
> 
> For example, in here:
> 
>    intptr_t v = CASPTR(&_LockWord, 0, _LBIT);  // agro ...
> 
> the type of the operand LockWord is SplitWord.  But the SplitWord *
> argument gets converted to void* volatile* when we call this:
> 
>     inline static void*        cmpxchg_ptr(void*        exchange_value, volatile void*         dest, void*        compare_value, cmpxchg_memory_order order = memory_order_conservative) {
>      return cmpxchg(exchange_value,
>                     reinterpret_cast<void* volatile*>(dest),
>                     compare_value,
>                     order);
> 
> Here's what I first wrote:
> 
>    I don't see the point of such a type conversion.  We could call
>    cmpxchg with the actual types of the operands, could we not?  Why is
>    cmpxchg_ptr even a thing?  We're casting away type information for
>    no reason that I can see.

While I'd be happy to see cmpxchg_ptr go away, my understanding was that 
it was needed in the API precisely to avoid (where possible) casts at 
the call site. I find it very frustrating that we have to jump through 
so many hoops to get the compiler to get out of the way. The atomic ops 
work on bits not semantics types - we only need to know how many bits 
we're dealing with, we don't care about pointers or ints or longs etc. 
The only semantics we need are that we check for a value that is valid 
for the destination to hold, and we provide a new value that is valid 
for the destination to hold. We know what those values are. The compiler 
doesn't and continually gets in our way because of that - and so we have 
to jump through hoop to persuade the compiler what we are doing is okay.

End of rant. :)

Cheers,
David
-----


>    Couldn't cmpxchg_ptr() be defined as a template function in such a
>    way that only the back ends that actually need to cast away the
>    types have to do so?  That is, if the back ends can define
>    cmpxchg_ptr() themselves without resorting to pointer type
>    conversion, we should let them so so.
> 
> But rather than sending that message straight away, I tried it.  And
> now I see: the compiler can't get the types right in those cases where
> we have mismatched operand types in the call.  Argh.  The only way we
> can get method resolution to work is to throw way the pointer type
> information and use void* for everything.  At th erisk of being
> boring, I repeat what I said before: IMO this is not what we should be
> doing in 2017.  We should be looking to the future, and get the types
> to match now, at the call site.
> 
> Also, and this is a relatively slight objection, I find myself
> defining
> 
> template<>
> struct Atomic::PlatformCmpxchg<1> VALUE_OBJ_CLASS_SPEC {
>    template<typename T>
>    T operator()(T exchange_value,
>                 T volatile* dest,
>                 T compare_value,
>                 cmpxchg_memory_order order) const {
>      return ::cmpxchg(exchange_value, dest, compare_value, order);
>    }
> };
> 
> for 1, 4, and 8.  I guess that can't be avoided, and in any case it
> would be easy enough to do it with preprocessor macros.
>