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

[Volker Simonis]

On Mon, Mar 11, 2019 at 6:15 PM Doerr, Martin <martin.doerr at sap.com> wrote:
>
> 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 see. it's unrelated to the actual issue but it makes sense to fix it
once we touch the file anyway.

> > 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.
>

OK. Thumbs up from me.

> 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
> >
> >