Request for reviews (S): 7190310: Inlining WeakReference.get(), and hoisting $referent may lead to non-terminating loops

[Vladimir Kozlov]

You don't need cpu order as David said. I simple did not find "software barrier" 
in C1. But I may not need it, see my discussion with Roland.

Vladimir

Vitaly Davidovich wrote:
> Actually, maybe this is because code is shared with G1 SATB pre barrier, 
> which probably does need ordering here of the load.
> 
> Sent from my phone
> 
> On Aug 16, 2012 8:19 AM, "Vitaly Davidovich" <vitalyd at gmail.com 
> <mailto:vitalyd at gmail.com>> wrote:
> 
>     Sorry, David corrected me - my question is for Vladimir not Roland.
>     :) sorry guys
> 
>     Sent from my phone
> 
>     On Aug 16, 2012 8:14 AM, "Vitaly Davidovich" <vitalyd at gmail.com
>     <mailto:vitalyd at gmail.com>> wrote:
> 
>         I agree, I don't think any cpu order/fence is needed here.
> 
>         Roland, what scenario are you envisioning that would require a
>         load fence at the CPU level?
> 
>         Thanks
> 
>         Sent from my phone
> 
>         On Aug 16, 2012 6:47 AM, "David Holmes" <david.holmes at oracle.com
>         <mailto:david.holmes at oracle.com>> wrote:
> 
>             On 16/08/2012 7:08 PM, Roland Westrelin wrote:
> 
>                 Hi Vladimir,
> 
>                     I updated webrev:
> 
>                     http://cr.openjdk.java.net/~__kvn/7190310/webrev.01
>                     <http://cr.openjdk.java.net/~kvn/7190310/webrev.01>
> 
>                     During testing I found several problem with code for
>                     unsafe.getObject() intrinsic code in both: C1 and
>                     C2. I moved some checks in C1 from
>                     G1UnsafeGetObjSATBBarrierStub stub into shared code
>                     and also added static klass analysis to avoid
>                     generation of G1/membar code if klass is not
>                     Reference or Object. In C2 I hit bad graph problem
>                     because If node's projections in
>                     insert_pre_barrier() were not put on IGVN worklist.
> 
>                     I added jtreg tests (thanks to John Cuthbertson for
>                     unsafe test which I extended).
> 
>                     Tested with CTW and Kitchensink with C1, C2, tiered.
> 
> 
>                 The c1 side doesn't look right to me.
> 
>                 insert_mem_bar(Op___MemBarCPUOrder) is not the same as
>                 __ membar_acquire(). __ membar_acquire() actually emits
>                 instructions on some platforms. Also, optimizations in
>                 c1 are performed at the HIR level. So adding the barrier
>                 at the LIR won't prevent any further optimization.
> 
> 
>             Not that I know much about C1 but the problem with C2 was
>             purely a compiler issue, hoisting the load. There should not
>             be any need to emit actual architectural level memory
>             barriers here.
> 
>             David
>             -----
> 
>                 Can this problem actually happen with c1?
> 
>                 Roland.
>