Release store in C2 putfield

Thu Sep 4 12:19:13 UTC 2014

On 04/09/14 12:30, Andrew Haley wrote:
> On 09/04/2014 10:30 AM, Bertrand Delsart wrote:
>> I'm not a C2 expert but from what I have quickly checked, an issue may
>> be that we need StoreStore ordering on some platforms.
>>
>> This should for instance be true for cardmarking (new stored oop
>> must be visible before the card is marked).
>
> Okay.  I can live with that.  Is there a corresponding read barrier in
> the code which scans the card table?

See for instance the storeload() in 
G1SATBCardTableLoggingModRefBS::write_ref_field_work

There are in fact a lot of other barriers in concurrent card scanning 
and cleaning (some of them being implicit due to compare and swap 
operations).

>
>> This may also be true for the oop stores in general, as initially
>> discussed. [IMHO this is related to final fields, which have to be
>> visible when the object escape. Barriers are the end of the
>> constructors may not be sufficient if objects can can escape before
>> the end of their init() method.
>
> I'm pretty sure we do this correctly.  Are you aware of any place
> (except unsafe publication, which is a programmer error) where this
> might happen?  We generate a barrier at the end of a constructor if
> there is a final field and at the end of object creation.

I agree that this is not a good programming style but I'm not sure this 
can always be considered a programmer error. Do you see anything in the 
java specification that forbid publication before the end of object 
creation ?

For instance, objects may have to be linked at creation time. In 
general, the publication should be safe because it will hit barriers 
(because what the object is exported too will often needs to be 
protected). However, I do not think this is mandatory according to the 
specifications.

Now, the problem is to see what the JMM requires in that case. I'm not 
100% sure that a StoreStore is needed here. This is why I said "may not 
be sufficient". The JSR-133 cookbook has several "(outside of 
constructor)" statements that might mean it is not needed (if you have a 
membar at the end of the constructor). However, while I'm familiar with 
barriers because of my runtime, GC and embedded background, I do not 
consider myself to be a JMM expert. I will let one chime in.

Of course, from a support point of view, it may be easier to add a 
StoreStore semantic on oop stores (should not be too expensive, taking 
into account the cost of GC barriers cost and the frequency of oop 
store) than to investigate the kind of troubles a strange ordering can 
lead to and explain to the customer why his Java code must be changed 
for platforms with weaker memory models. Did you measure the performance 
regression ?

Bertrand.

>
> I should explain: I respect all nodes, and generate all barriers,
> except when there is a redundant barrier.  So, if a barrier is
> immediately preceded by a load acquire I don't emit anything; likewise
> for a store release.
>
> So, for something like
>
>      volatile int x;
>
>      void foo() {
>          while (true) x = 1;
>      }
>
> I get
>
>    ;; membar_release (elided)
>    0x0000007fa822f1e4: add	x10, x19, #0xc
>    ;; 0x1
>    0x0000007fa822f1e8: orr	w12, wzr, #0x1
>    0x0000007fa822f1ec: stlr	w12, [x10]
>    0x0000007fa822f1f0: dmb	ish             ;*putfield x
>                                                  ; - VolatileStore::foo at 2 (line 5)
>
>    0x0000007fa822f1f4: adrp	x11, 0x0000007fb7ff7000
>                                                  ; OopMap{r19=Oop off=120}
>                                                  ;*goto
>                                                  ; - VolatileStore::foo at 5 (line 5)
>                                                  ;   {poll}
>    0x0000007fa822f1f8: ldr	wzr, [x11]      ;*goto
>                                                  ; - VolatileStore::foo at 5 (line 5)
>                                                  ;   {poll}
>    0x0000007fa822f1fc: b	0x0000007fa822f1e4
>
> Which is correct, I think.
>
>> Unfortunately, MemNode only defines higher level 'release' and
>> 'acquire'. This means that if you want to use MemNode to guarantee the
>> StoreStore (instead of using a separate membar), then you need to use
>> MemNode::release... which uselessly adds the LoadStore semantic.
>>
>> LoadStore may require a stronger barrier (for instance "DMB SY" instead
>> of "DMB ST" on ARM32).
>>
>> We have the same issue with some other code in hotspot. Some StoreStore
>> constraints just before a write have been implemented using
>> OrderAccess::release_store(). The later is currently slightly more
>> efficient on SPARC/x86... but this is because OrderAccess::storestore()
>> is not fully optimized (we just need to prevent C++ compiler reordering,
>> we should not have to actually generate code for storestore on TSO
>> systems). Unfortunately, on platforms with weaker memory models,
>> release_store() may be less efficient than a storestore() followed by a
>> write :-(
>
> Indeed.  I think the back end should drive this.
>
>> May be we need in OrderAccess and in MemNode a new store operation
>> weaker than release_store, ordering only the store wrt previous stores
>> (e.g. "membar #StoreStore; write"). This will be a simple store on TSO
>> systems (+ something to prevent compiler reordering) but will be
>> expanded to whatever is the most efficient barrier on the other systems.
>
> It would help back-end writers tremendously if there were more kinds
> of MemNodes, preferably annotated in such a way that we could tell
> what each barrier was for.  That way we could tell whether this is a
> barrier at the end of object creation, etc.  And I could simply not
> generate anything for barriers that are unneeded because of store
> release instructions.
>
> Andrew.
>

-- 
Bertrand Delsart,                     Grenoble Engineering Center
Oracle,         180 av. de l'Europe,          ZIRST de Montbonnot
38334 Saint Ismier,                                        FRANCE
bertrand.delsart at oracle.com             Phone : +33 4 76 18 81 23

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: This email message is for the sole use of the intended
recipient(s) and may contain confidential and privileged
information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient,
please contact the sender by reply email and destroy all copies of
the original message.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~