RFR (S): 8153751: Windows Atomic and OrderAccess are not emitting explicit compiler barriers

[David Holmes]

Hi Erik,

Sorry for the delay on this.

On 8/04/2016 7:00 PM, Erik Österlund wrote:
> Hi,
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8153751
> Webrev: http://cr.openjdk.java.net/~eosterlund/8153751/webrev.00/
>
> It turns out that on Windows, compiler barriers are not used for
> Atomic:: code and some code in OrderAccess. It is assumed that the
> non-volatile accesses will not be reordered across __asm statements, as
> the current memory model should prevent. But I have not found any
> guarantees that this is the case for __asm.

Agreed - not that I would expect it to be clearly documented even if 
true. There is no reason to assume that any arbitrary _asm statement 
automatically acts as a compiler barrier, though some may.

> The closest source I could find is the documentation for the
> MemoryBarrier macro that uses an __asm statement to produce a hardware
> membar. The documentation says the following: "Creates a hardware memory
> barrier (fence) that prevents the CPU from re-ordering read and write
> operations. It may also prevent the compiler from re-ordering read and
> write operations."
>
> Note the "MAY" prevent compiler reordering. (*facepalm*) This sounds to
> me like __asm really does not guarantee compiler reordering across __asm
> at all.

Yeah that sounds like it was written by someone who really didn't 
understand the issues. :(

> A suspected victim of this reordering is here:
> https://bugs.openjdk.java.net/browse/JDK-8033545

I don't really see how that particular bug would be helped by additional 
compiler-barriers. As I understand the report it was the elision of a 
non-volatile load that caused the problem, not a reordering. But anyway ...

> To make the use of these classes more safe, I propose to simply add
> explicit compiler barriers in this windows code. It should not make any
> difference, as I am only enforcing compiler reordering rules that should
> already hold.

Okay, I agree with the proposed changes - the CompilerBarrierScope 
should effectively have no impact on generated code, other than 
prohibiting code motion.

> This small patch fixes this. The only thing to note is that the __asm
> statements that put things in %eax and avoid returning anything.

Could you elaborate on that please ??

> Testing: JPRT

Did you compare the generated object code before and after to see if it 
actually changed anything?

Thanks,
David
-----

> Thanks,
> /Erik