More on memory barriers

[Vitaly Davidovich]

Looking at hg history, MemBarStoreStore was added a few years ago, whereas
the code in question is much older.  The comments in the changelist adding
MemBarStoreStore seem to indicate it was done to address a specific issue,
and my guess is that it wasn't "retrofitted" into all possible places.
 Just a guess though ...

On Mon, Sep 15, 2014 at 12:13 PM, Andrew Haley <aph at redhat.com> wrote:

> On 09/15/2014 05:08 PM, Doug Lea wrote:
> > On 09/15/2014 11:20 AM, Andrew Haley wrote:
> >> I'm still looking at the best way to generate code for AArch64
> volatiles,
> >> and I've come across something really odd.
> >>
> >> void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
> >>    bool is_vol = field->is_volatile();
> >>    // If reference is volatile, prevent following memory ops from
> >>    // floating down past the volatile write.  Also prevents commoning
> >>    // another volatile read.
> >>    if (is_vol)  insert_mem_bar(Op_MemBarRelease);
> >>
> >> A release here is much too strong: the JSR-133 cookbook says that we
> >> only need a StoreStore here, presumably because we're going to emit a
> >> full StoreLoad barrier after the store to the volatile field.
> >>
> >> On a target where this makes an actual difference to code quality,
> >> this matters.  Does anyone here understand what this release fence is
> >> for?
> >
> > My understanding is that storeStore was not distinguished from
> > release only because it did not impact any existing platforms. (Plus
> > as I mentioned before, storeStore is unlikely to ever be exposed
> > as part of any user-visible mode).
>
> Okay, that makes sense.
>
> > But I can't see any reason not to define a Op_MemBarStoreStore as a
> > subtype of Op_MemBarRelease, keeping everything the same in c2, but
> > matching it more cheaply on platforms where it matters. (You lose
> > almost nothing, or maybe exactly nothing treating it as release wrt
> > other c2 internals.)
> >
> > [Insert my usual disclaimers about not being a hotspot engineer.]
>
> Mmm.  MemBarStoreStore is defined, it's just not used for this
> operation.
>
> Andrew.
>