RFR (M): 8184334: Generalizing Atomic with templates

[Andrew Haley]

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.

  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.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671