missing memory barrier in acmp with C2

[Andrew Haley]

On 26/10/16 16:31, Vitaly Davidovich wrote:
> On Wednesday, October 26, 2016, Andrew Haley <aph at redhat.com> wrote:
> 
>> On 26/10/16 15:02, Vitaly Davidovich wrote:
>>> On Wednesday, October 26, 2016, Andrew Haley <aph at redhat.com
>> <javascript:;>> 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?
> 
> I don't follow - x is an oop, and x.blah is at (addr of x) + (offset of
> blah field).  You need to load addr of x 

Where do you suppose the addr of x is being loaded from?

The addr of x is in a register already.  We don't need to read it
from a field.  It may be an argument, for example.

> to figure out dest addr of the store.  As written in your snippet,
> the load of x is after the loadload.

It's not.

>>> 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.
> 
> As mentioned above, to get x.blah address you need a load of x (or have the
> address available already) - that's data dependent load.

Dependent on what?

Andrew.