review for 7032963: StoreCM shouldn't participate in store elimination

[Tom Rodriguez]

On Apr 1, 2011, at 3:47 PM, Vladimir Kozlov wrote:

> You may put n->in(MemNode::Address) and n->in(MemNode::ValueIn) into locals before the loop. Also you need to kill the node explicitly otherwise it still be connected to its inputs:
> 
> +         // Eliminate the previous StoreCM
> +         prev->set_req(MemNode::Memory, mem->in(MemNode::Memory));
> +         assert(mem->outcnt() == 0, "should be dead");
> +         mem->disconnect_inputs(NULL);

I'll have to rework the mem traversal a little.  Actually I think there might have been a bug with the old code since it always updated prev.  I believe this is correct:

        // Eliminate the previous StoreCM                                                                                                             
        prev->set_req(MemNode::Memory, mem->in(MemNode::Memory));
        assert(mem->outcnt() == 0, "should be dead");
        mem->disconnect_inputs(NULL);
      } else {                                                                                                                                       
        prev = mem;                                                                                                                                   
      }
      mem = prev->in(MemNode::Memory);
    }

I think I'll put together a little test case to make sure this is working correctly.

tom

> 
> Vladimir
> 
> Tom Rodriguez wrote:
>> I could push this to hotspot-gc so it gets more CMS testing .
>> http://cr.openjdk.java.net/~never/7032963
>> 7032963: StoreCM shouldn't participate in store elimination
>> Reviewed-by:
>> StoreCM shouldn't participate in redundant store elimination since
>> that could violate the requirement that a StoreCM must be strictly
>> after a field update.  This results in a large number of redundant
>> StoreCMs being emitted for blocks of fields updates, so I added an
>> optimization to fold them up safely.  Previously the extra dependence
>> was converted into a precedence edge just before register allocation
>> but I moved this logic into final_graph_reshape.  I then added logic
>> to search through chains of StoreCMs to eliminate earlier redundant
>> ones and transfer their precedence edges to the one that is kept.
>> This ensures that they are scheduled properly.  This actually
>> eliminates duplicates that were previously missed so the code quality
>> is slightly better.  Tested by inspecting code generation with script
>> to identify duplicates.  Also ran CTW with -XX:+UseCondCardMark and
>> -XX:+UseG1GC.