RFR: 8166651: OrderAccess::load_acquire &etc should have const parameters (original) (raw)

Kim Barrett kim.barrett at oracle.com
Thu May 25 19:47:02 UTC 2017

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/

More information about the hotspot-dev mailing list