Updated request for reviews (S): 6984348

[Vladimir Kozlov]

Tom Rodriguez wrote:
> Can you explain what problem the old code was checking for and how the new code handles that? 

The code was added to catch situation similar to 6896727
where MergeMemory node general memory slice was not
updated correctly since it was moved with moved store.
The changes for 6896727 fixed it by moving store's users
before the store change its memory (move_inst_mem() was added).

The original check was added to verify that moved stores
will be replaced by corresponding memory slice.
But relaying on number of output edges for the check
was not correct.

New asserts verify that old_mem output edges were
correctly updated. But looking on this more it
could be not enough or even correct.
Also missing (as before) checks for inputs update
of non-instance Phis (Phase 4 first loop).

> If m == orig_mem why do we use find_inst_mem to look it up?  Wouldn't it make more sense for find_inst_mem to be used only in the assert?

I am concern that could be cases when m != orig_mem.
Consider the case when old_mem is a store into another
instance. Then it will be not general memory slice.
But I can't find such cases with testing.

I need to spend more time on this.

Thanks,
Vladimir

> 
> tom
> 
> On Sep 15, 2010, at 11:15 AM, Vladimir Kozlov wrote:
> 
>> http://cr.openjdk.java.net/~kvn/6984348/webrev.01
>>
>> Fixed 6984348: Fix typo in escape.cpp
>>
>> After fixing typo the assert produces false negative results.
>> It does not take into account that Loads don't have output
>> memory edge and Stores may have several memory users.
>>
>> I removed this code and added few asserts into move_inst_mem().
>>
>> Tested with CTW and NSK.
>>
>