RFR: 8166651: OrderAccess::load_acquire &etc should have const parameters

[Kim Barrett]

> On May 25, 2017, at 11:54 AM, Andrew Dinn <adinn at redhat.com> wrote:
> 
> Ok, so I have several interesting things to report.

Thanks for trying this out, and apologies for the blunders.

I got the impression that casting away constness would be needed here
from the documentation, which has the signature as

  __atomic_load(type *ptr, type *ret, int memorder)

The lack of any mention of const, and that "type" can't be optionally
const in both places (since ret *is* written to), led me to that
apparently erroneous conclusion.

I could have saved us both some trouble if I'd thought of simply
experimenting with it on some platform that I *do* have access to.  A
little experimenting today (on x86_64) suggests the documentation is
misleading.  Sorry about that.

Regarding the load_ptr_acquire problem, that's happening because
const_cast can *only* modify the cv-qualifiers, and can't affect the
non-cv part of the type.  So my error here.  In my (weak) defense, I
try not to write code that needs const_cast, so haven't used it much
and simply forgot its protocol.  And this is another case I could have
found by experimenting on another platform.

It looks like I also forgot to remove one of the now duplicate
load_ptr_acquire variants for aarch64 too.

Strangely, what should be an equivalent variation of your suggested
load_ptr generates a spurious compiler warning when I try it with
gcc4.9 on x86_64.  If the static_cast is captured in a variable, as in:

void* foo(const volatile void* p) {
  void* data;
  void* const volatile* pp = static_cast<void* const volatile*>(p);
  __atomic_load(pp, &data, __ATOMIC_ACQUIRE);
  return data;
}

I get this warning:
  warning: variable ‘pp’ set but not used [-Wunused-but-set-variable]

although the proper code gets generated.  I have no explanation for
this.  But it doesn't matter for now, since we don't need to use that
form.  In fact, the old version of load_ptr_acquire(const volatile
void*) works fine, so changes to it have been reverted.  I've only
removed the unneeded load_ptr_acquire(volatile void*) overload.

And regarding my question about __atomic_load vs __atomic_load_n, at
least for x86_64 the same code gets generated.  Probably the same is
true for aarch64.

So here's the new webrev.  The only changes are in
orderAccess_linux_aarch64.inline.hpp, which I can't test.  Hopefully
I've not made any more blunders.

http://cr.openjdk.java.net/~kbarrett/8166651/hotspot.01/