RFR (S): 8007898: Incorrect optimization of Memory Barriers in Matcher::post_store_load_barrier()

[Doerr, Martin]

Hi Vladimir,

thanks for fixing these problems. The change looks good.

There's only one question remaining. We're not sure if the following pattern can occur:

MemBarVolatile
 ^                   ^
 |                   |
CtrlProj    MemProj
 ^                   ^
 |                   |
 |                 Load
 |
MemBarVolatile

If this can occur, we will have to traverse the memory edges in addition.
We have implemented a similar optimization to optimize MemBarAcquire before MemBarVolatile away
and we had to check the memory edges. An excerpt of our code is below.

Kind regards,
Martin and Goetz



  // Get the Proj node, mem_proj, that can be used to iterate forward
  Node *mem_proj = NULL;
  DUIterator_Fast imax, i = mem->fast_outs(imax); 
  while( true ) {
    mem_proj = mem->fast_out(i);               // Throw out-of-bounds if proj not found
    assert( mem_proj->is_Proj(), "only projections here" );
    ProjNode *proj = (ProjNode*)mem_proj;
    if( proj->_con == TypeFunc::Memory &&
        !C->node_arena()->contains(mem_proj) ) // Unmatched old-space only
      break;
    i++;
  }

  bool found_membar = false;
  for( DUIterator_Fast jmax, j = mem_proj->fast_outs(jmax); j < jmax; j++ ) {
    Node *x = mem_proj->fast_out(j);
    // Proj might have an edge to a store or load node which precedes the membar
    if (x->is_Mem()) return false;

    // We can only allow xop == Op_MemBarVolatile when we traverse TypeFunc::Memory instead of TypeFunc::Control
    // because a LoadNode might get scheduled before the MemBarVolatile when considering TypeFunc::Control only.
    int xop = x->Opcode();
    if (xop == Op_MemBarVolatile) {
      // Make sure we're not missing Call/Phi/MergeMem by checking control edges.
      // That means we only set found_membar=true when x can be found via control proj AND memory proj.
      Node *in = x->in(0);
      if (in->is_Proj() && in->in(0)==vmb) found_membar = true;
    }
  }

  return found_membar;




-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Donnerstag, 11. Juli 2013 05:39
To: hotspot compiler
Cc: Aleksey Shipilev; Doerr, Martin; Lindenmaier, Goetz
Subject: RFR (S): 8007898: Incorrect optimization of Memory Barriers in Matcher::post_store_load_barrier()

http://cr.openjdk.java.net/~kvn/8007898/webrev/

The problem is caused by complicated (and questionable) barriers code 
generated for volatile stores. It is questionable from performance view 
because second volatile membar is for bottom memory (AliasIdxBot). So 
any memory operations can not pass it.
The fix is to generate one "fat" membar instead of set of barriers.

I also cleaned/fixed corresponding code in Matcher which tries to 
optimize out those multiply barriers. The problem with it was it did not 
take into account biased locking when cas could be not executed.
Also code for FastUnlock ('If' check) was not removed before because of, 
I think, incorrect comment "upcoming fastlock block" - it should be 
"fastUnlock block".

Added regression test which shows the problem with -XX:+StressLCM[GCM] 
flags added by Aleksey some time ago.

Aleksey is testing performance of these changes and no regression is 
observed so far.

Thanks,
Vladimir