RFR(S): 8220441: [PPC64] Clobber memory effect missing for memory barriers in atomics

[Doerr, Martin]

Hi Volker,

thank you for reviewing.

> Why did you had to include "utilities/debug.hpp" in
> atomic_linux_ppc.hpp?
STATIC_ASSERT is used which is in that file. We'd risk build errors when changing other includes without it.

> I also saw that we have Prefetch::read()/Prefetch::write() ...
They don't modify memory, so it's not a problem to omit the "clobber memory" for them.
These 2 functions are only there to possibly improve performance.
"clobber memory" may have a negative performance impact for them.

I don't have performance concerns in the case of the atomic memory barriers because the immediately following or preceding inline assembler code pieces already have that effect, so C++ compilers have to be conservative anyway.

Best regards,
Martin


-----Original Message-----
From: Volker Simonis <volker.simonis at gmail.com> 
Sent: Montag, 11. März 2019 17:36
To: Doerr, Martin <martin.doerr at sap.com>
Cc: hotspot-runtime-dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Subject: Re: RFR(S): 8220441: [PPC64] Clobber memory effect missing for memory barriers in atomics

Hi Martin,

thanks for fixing this.

Why did you had to include "utilities/debug.hpp" in
atomic_linux_ppc.hpp ? Is it really necessary or just a leftover?

I also saw that we have Prefetch::read()/Prefetch::write()
src/hotspot/os_cpu/aix_ppc/prefetch_aix_ppc.inline.hpp and
src/hotspot/os_cpu/linux_ppc/prefetch_linux_ppc.inline.hpp. Do they
also have a memory effect?

Besides that the change looks good!

Regards,
Volker

On Mon, Mar 11, 2019 at 5:05 PM Doerr, Martin <martin.doerr at sap.com> wrote:
>
> Hi,
>
>
>
> the atomics in atomic_aix/linux_ppc.hpp use inline assembler memory barriers. They need a "clobber memory" effect in order to get handled correctly by C++ compilers.
> We haven't seen issues so far, but we should avoid problems with future compiler versions.
>
>
>
> Please review
>
> http://cr.openjdk.java.net/~mdoerr/8220441_ppc_clobber_mem/webrev.00/
>
>
>
> Best regards,
>
> Martin
>
>