RFR(M): Memory ordering in taskqueue.hpp

[Lindenmaier, Goetz]

Hi David,

why do you think only these two accesses have to be ordered?

As I understand, any two accesses in one thread to these fields have to be 
orderered and must be written to memory to become visible in the proper order 
to other threads.  The PPC compiler really takes all advantages given by the 
C-Standard to optimize this, and the processor reorders heavily.

The specification of volatile in 
       Age get() const volatile { return _data; }
says that it must be read from memory, not that it can not be reordered.

Also, doing 
      OrderAccess::load_acquire((volatile idx_t*) &(_age._fields._top))
is better wrt. to optimizations by C-compilers, as there first 
is the whole address computation, and then the cast to volatile. 
If we use
      _age.top()
with 
      idx_t top() const volatile { return OrderAccess::load_acquire((volatile idx_t*) &(_fields._top); }
compilers don't optimize that well.  (We saw this on hpia64.)
Further, the access to, e.g., old_age.top() in pop_local_slow() would do 
the OrderAccess, which is useless overhead.

Best regards,
  Goetz.


-----Original Message-----
From: David Holmes [mailto:david.holmes at oracle.com] 
Sent: Dienstag, 18. Dezember 2012 01:24
To: Lindenmaier, Goetz
Cc: hotspot-dev at openjdk.java.net
Subject: Re: RFR(M): Memory ordering in taskqueue.hpp

Hi Goetz,

On 17/12/2012 10:58 PM, Lindenmaier, Goetz wrote:
> Hi,
>
> Once more, with webrev:
> http://cr.openjdk.java.net/~goetz/webrevs/webrev-taskqueue/

I can see that this function needs a storestore barrier:

  237   void set_empty() {
  238     _bottom = 0;
  239     _age.set(0);
  240   }

to preserve ordering. Are there other functions to which we can 
constrain the loadload and storestore barriers rather than using the 
setters/getters the way you have defined? This is always about a pair 
(sometimes groups) of accesses so I'd rather deal with the pairs than 
treat each field as if it were a Java volatile.

Thanks,
David Holmes


> Sorry for that.
>
> I would like to contribute fixes wrt. memory ordering in taskqueue.hpp.
>
> On Platfoms with weak memory ordering the taskqueue is not accessed
> properly, as the accesses to the fields _bottom and _age are not ordered
> correctly. Volatile is not sufficient to enforce this, because it depends on
> what the compiler assumes to be necessary for volatile variables.
>
> The fix is to pull getter/setter routines from Age to TaskQueueSuper and use methods from
> OrderAccess to access the fields in _age as well as _bottom.   Further the code must always
> use the getter/setter methods to access _bottom, _age or the subfields of _age.
> On the other hand we can relax constraints for accesses to locals oldAge and newAge.
>
> The OrderAccess routines do simple load/stores on x86_64.
>
> I want to discuss this change here and would like very much to see it taking
> it's way into OpenJDK to support ports on platforms with weak memory
> ordering, and, in particular, our PPC port.
>
> Best regards,
>    Goetz.
>
>