Request for reviews (L): 7125896: Eliminate nested locks

[Vladimir Kozlov]

Tom Rodriguez wrote:
>>
>>> Why can't the BoxLockNode be modified in place instead of being replaced?  They are no longer shared between lock regions.  Actually if sharing of them is disabled then you can always just modify them in place can't you?
>> BoxLockNode could be merged so lock->box_node() could be PhiNode or it could be used by PhiNode.
> 
> Why does that happen?  OSR? 

Not necessary OSR but some weird bytecode (there was bug recently with huge 
method which had a lot inlining, looping and strange merge points). Compiler 
also does partial peeling and split Ifs which may create Phis, I think.

I ran full CTW and compiler regression tests and did not hit BoxLockNode merge 
case so it could be very rare case. But I want to be careful.

> Does lock elimination require that the Phis be collapsed?

No, that is why it clones BoxLockNode.

> 
>> I do modify original BoxLockNode for nested case where merged cases are excluded. For eliminated by EA case I could do cloning only for merged cases but it needs additional checks so I decided to do it always. But if merged cases are very rare then cloning should be avoided for not merged BoxLock, I will check.
> 
> At a minimum, can't the explicit iteration over all the users can be replaced with igvn.replace_node(oldbox, newbox)?  The explicit iteration seems like overkill.

I want to play it safe and replace BoxLock only for related users (same box and 
object and not Phi). It is matching old elimination code so I will move it to 
separate method and will use in both cases.

Thanks,
Vladimir

> 
>>> eliminating the cloning in ciTyepFlow could have performance implications.  I assume it's rare?
>> I ran refworkload on x86 and SPARC, there was no change in scores. For normal loop's head cloning cases monitorenter can not be in first loop's block (there will be condition there). The case I hit was "while(true) { synchronize(o);" where there is no condition. So it is not common case. Also LockNode is Call node and we don't do much loop optimizations for loops with calls inside.
> 
> Ok.
> 
> tom
> 
>> Thanks,
>> Vladimir
>>
>>> tom
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>> -- Chris
>>>>> On Dec 29, 2011, at 10:20 PM, Vladimir Kozlov wrote:
>>>>>> http://cr.openjdk.java.net/~kvn/7125896/webrev
>>>>>>
>>>>>> 7125896: Eliminate nested locks
>>>>>>
>>>>>> Nested locks elimination done before lock nodes expansion by looking for outer locks of the same object. Commoning (GVN) of BoxLock nodes is switched off because nested locks elimination requires separate BoxLock node for each locked region to generated correct debug info for deoptimization. As result there could be merges (and Phi nodes) of BoxLock nodes. One such merge generated by ciTypeFlow (cloning loop head) is avoided but there could be other cases so new code is added to handle it.
>>>>>>
>>>>>> New code is under new product flag EliminateNestedLocks.
>>>>>>
>>>>>> Also added missed KILL effect for box register in fastlock and fastunlock mach nodes definitions.
>>>>>>
>>>>>> Tested with full CTW, nsk, jtreg tests, refworkload.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>