missing memory barrier in acmp with C2

[Andrew Haley]

On 26/10/16 15:02, Vitaly Davidovich wrote:
> On Wednesday, October 26, 2016, Andrew Haley <aph at redhat.com> wrote:
> 
>> On 26/10/16 12:27, Roman Kennke wrote:
>>> Am Mittwoch, den 26.10.2016, 13:24 +0200 schrieb Roland Westrelin:
>>>> http://cr.openjdk.java.net/~roland/shenandoah/membar-acmp/webrev.00/
>>>>
>>>> The code generated for acmp is missing a memory barrier.
>>>
>>> Great!
>>>
>>>> Should it be a loadstore + loadload as in
>>>> ShenandoahBarrierSet::asm_acmp_barrier() on aarch64 or simply a
>>>> loadload?
>>>
>>> I can come up with a reason for loadload, but not for loadstore, I
>>> think loadstore is not necessary there. I'd go for the less restrictive
>>> fence unless we come up with a good reason not to.
>>
>> The general rule is that you can get away with loadload fences if you
>> really know what you are doing, but it is exceedingly subtle.
>>
>> Imagine this.  We have two variables, a boolean x_init and an oop
>> x.
>>
>> Thread 1:
>> <Initialize x>
>> x_init.store_release(true);
>>
>> Thread 2:
>> if (x_init.load_aquire())
>>     x.blah = y
>>
>> If you replace the load acquire with a loadload fence, the store of
>> x.blah can become visible before the initialization of x.
> 
> x.blah requires a load of x (which cannot reorder with loadload)

x is just a local, and it's in a register.  Where would you even load
it from?

> and it's data dependent; unless you take something like Alpha into
> account, but that's unsupported anyway.

Please explain.  And, while you're at it, please explain why Hans is
wrong, or why my interpretation is wrong.

Andrew.